You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@johnzon.apache.org by "SingingBush (via GitHub)" <gi...@apache.org> on 2023/01/26 01:59:08 UTC
[GitHub] [johnzon] SingingBush opened a new pull request, #99: JOHNZON-305 : Java Modules
SingingBush opened a new pull request, #99:
URL: https://github.com/apache/johnzon/pull/99
now that Java 11 is the minimum adding proper support for JPMS using a _module-info.java_ instead of specifying _Automatic-Module-Name_ is preferable. Unfortunately this will involve a fair amount of refactoring so this PR is a mix of both approaches.
As mentioned in [JOHNZON-305](https://issues.apache.org/jira/browse/JOHNZON-305), getting the _Automatic-Module-Name_ configured using maven-jar-plugin was not working due to the bundle plugin. I ended up setting the auto module name using the maven-bundle-plugin itself in most cases.
The core module is done properly with a _module-info.java_ but adding that to all the maven modules won't work without a fairly large PR. I'd like to know if there's interest in merging this before making further changes.
In the projects that I've done previously with JPMS I've generally gone with the approach of putting unit tests into a different package name to prevent the split packages problem. For example, all the tests under *org.apache.johnzon.core* I'd change the package to something like *ut.johnzon.core*. That way a _module-info.java_ can be defined in the main source and the test source. I noticed there are some tests in Johnzon that rely on test classes being in the same package as source files to get access to package private code. That would need some re-work.
Getting _johnzon.core_ working with JPMS was fairly straight forward. I have a module info locally for _johnzon.mapper_ that essentially works when compiling the main source but could not include it in the PR due to _JsonGeneratorCloseTest_ using test code in the _org.apache.johnzon.core_ package. That would be along the lines of:
```
module johnzon.mapper {
requires java.desktop; // for java.beans
requires jakarta.cdi;
requires jakarta.json;
requires jakarta.persistence;
requires jakarta.transaction;
requires johnzon.core;
opens org.apache.johnzon.mapper.internal to johnzon.jaxrs;
exports org.apache.johnzon.mapper;
exports org.apache.johnzon.mapper.access;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [johnzon] rmannibucau commented on a diff in pull request #99: JOHNZON-305 : Java Modules
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on code in PR #99:
URL: https://github.com/apache/johnzon/pull/99#discussion_r1087663108
##########
johnzon-jsonb/pom.xml:
##########
@@ -144,6 +144,7 @@
<artifactId>maven-bundle-plugin</artifactId>
<configuration>
<instructions>
+ <Automatic-Module-Name>johnzon.jsonb</Automatic-Module-Name>
Review Comment:
Well module-info or automatic name but all modules should have the same option (it is the key). module-info has some pitfalls and will break some integration - why we dont have it yet - but since we move to 2.0 very soon I guess door can be opened now if preferred but clearly a single solution for the full project :pray: .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [johnzon] SingingBush commented on a diff in pull request #99: JOHNZON-305 : Java Modules
Posted by "SingingBush (via GitHub)" <gi...@apache.org>.
SingingBush commented on code in PR #99:
URL: https://github.com/apache/johnzon/pull/99#discussion_r1087650145
##########
johnzon-jsonb/pom.xml:
##########
@@ -144,6 +144,7 @@
<artifactId>maven-bundle-plugin</artifactId>
<configuration>
<instructions>
+ <Automatic-Module-Name>johnzon.jsonb</Automatic-Module-Name>
Review Comment:
yes using a module-info file is a better option if possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [johnzon] SingingBush commented on a diff in pull request #99: JOHNZON-305 : Java Modules
Posted by "SingingBush (via GitHub)" <gi...@apache.org>.
SingingBush commented on code in PR #99:
URL: https://github.com/apache/johnzon/pull/99#discussion_r1087860443
##########
johnzon-core/src/main/java/module-info.java:
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+module johnzon.core {
Review Comment:
sure no problem, so for this one I'll update it to be `org.apache.johnzon.core` and the same convention for the others
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [johnzon] rmannibucau commented on a diff in pull request #99: JOHNZON-305 : Java Modules
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on code in PR #99:
URL: https://github.com/apache/johnzon/pull/99#discussion_r1087512432
##########
johnzon-jaxrs/src/main/java/module-info.java:
##########
@@ -0,0 +1,32 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+module johnzon.jaxrs {
+ requires java.xml;
Review Comment:
This part is optional
##########
johnzon-core/src/main/java/module-info.java:
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+module johnzon.core {
Review Comment:
Please use org.apache in the naming
##########
johnzon-jsonb/pom.xml:
##########
@@ -144,6 +144,7 @@
<artifactId>maven-bundle-plugin</artifactId>
<configuration>
<instructions>
+ <Automatic-Module-Name>johnzon.jsonb</Automatic-Module-Name>
Review Comment:
To drop if we use module-info?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [johnzon] SingingBush commented on a diff in pull request #99: JOHNZON-305 : Java Modules
Posted by "SingingBush (via GitHub)" <gi...@apache.org>.
SingingBush commented on code in PR #99:
URL: https://github.com/apache/johnzon/pull/99#discussion_r1088721486
##########
johnzon-core/src/main/java/module-info.java:
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+module johnzon.core {
Review Comment:
done
##########
johnzon-jsonb/pom.xml:
##########
@@ -144,6 +144,7 @@
<artifactId>maven-bundle-plugin</artifactId>
<configuration>
<instructions>
+ <Automatic-Module-Name>johnzon.jsonb</Automatic-Module-Name>
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [johnzon] rmannibucau merged pull request #99: JOHNZON-305 : Java Modules
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau merged PR #99:
URL: https://github.com/apache/johnzon/pull/99
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [johnzon] SingingBush commented on a diff in pull request #99: JOHNZON-305 : Java Modules
Posted by "SingingBush (via GitHub)" <gi...@apache.org>.
SingingBush commented on code in PR #99:
URL: https://github.com/apache/johnzon/pull/99#discussion_r1087862814
##########
johnzon-jsonb/pom.xml:
##########
@@ -144,6 +144,7 @@
<artifactId>maven-bundle-plugin</artifactId>
<configuration>
<instructions>
+ <Automatic-Module-Name>johnzon.jsonb</Automatic-Module-Name>
Review Comment:
ok for this PR I'll keep it to simply using the _Automatic-Module-Name_ approach. With the goal of moving over to using a module-info file in the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@johnzon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org