You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "timtebeek (via GitHub)" <gi...@apache.org> on 2023/03/12 21:13:34 UTC
[GitHub] [wicket] timtebeek opened a new pull request, #556: WICKET-7029 Add wicket-migration module
timtebeek opened a new pull request, #556:
URL: https://github.com/apache/wicket/pull/556
Makes it easier for users to adopt Apache Wicket 10. https://issues.apache.org/jira/browse/WICKET-7029
Migration then comes down to executing a single command:
```shell
mvn -U org.openrewrite.maven:rewrite-maven-plugin:run \
-Drewrite.recipeArtifactCoordinates=org.apache.wicket:wicket-migration:LATEST \
-DactiveRecipes=org.apache.wicket.BestPractices
```
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on pull request #556: WICKET-7029 Add wicket-migration module
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #556:
URL: https://github.com/apache/wicket/pull/556#issuecomment-1481030482
Big thank you, @timtebeek !
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] timtebeek commented on pull request #556: WICKET-7029 Add wicket-migration module
Posted by "timtebeek (via GitHub)" <gi...@apache.org>.
timtebeek commented on PR #556:
URL: https://github.com/apache/wicket/pull/556#issuecomment-1466040011
There's still some dependency convergence errors; hoping the next OpenRewrite release clears at least a few of those up. Any others can then be fixed through exclusions or dependency management, unless you have other ideas there.
Apart from that I don't think there's much holding up this PR from getting merged and maintained with new breaking changes going forward.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on a diff in pull request #556: WICKET-7029 Add wicket-migration module
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #556:
URL: https://github.com/apache/wicket/pull/556#discussion_r1133596851
##########
pom.xml:
##########
@@ -119,7 +119,8 @@
<module>wicket-native-websocket</module>
<module>wicket-bean-validation</module>
<module>wicket-user-guide</module>
- </modules>
+ <module>wicket-migration</module>
Review Comment:
You are right!
The user can use `-DactiveRecipes=org.apache.wicket.MigrateToWicket10` instead of `-DactiveRecipes=org.apache.wicket.BestPractices` if (s)he wants to execute only the steps for a specific version.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] timtebeek commented on pull request #556: WICKET-7029 Add wicket-migration module
Posted by "timtebeek (via GitHub)" <gi...@apache.org>.
timtebeek commented on PR #556:
URL: https://github.com/apache/wicket/pull/556#issuecomment-1469740937
So the bump of the rewrite-recipe-bom helped; now there's only one enforcer rule violation left:
```
Warning:
Dependency convergence error for jakarta.activation:jakarta.activation-api:jar:1.2.2:runtime paths to dependency are:
+-org.apache.wicket:wicket-migration:jar:10.0.0-M1-SNAPSHOT
+-org.openrewrite.recipe:rewrite-migrate-java:jar:1.18.0:compile
+-org.openrewrite:rewrite-maven:jar:7.38.0:runtime
+-com.fasterxml.jackson.module:jackson-module-jaxb-annotations:jar:2.13.4:runtime
+-jakarta.xml.bind:jakarta.xml.bind-api:jar:2.3.3:runtime
+-jakarta.activation:jakarta.activation-api:jar:1.2.2:runtime
and
+-org.apache.wicket:wicket-migration:jar:10.0.0-M1-SNAPSHOT
+-org.openrewrite.recipe:rewrite-migrate-java:jar:1.18.0:compile
+-org.openrewrite:rewrite-maven:jar:7.38.0:runtime
+-com.fasterxml.jackson.module:jackson-module-jaxb-annotations:jar:2.13.4:runtime
+-jakarta.activation:jakarta.activation-api:jar:1.2.1:runtime
Error: Rule 1: org.apache.maven.plugins.enforcer.DependencyConvergence failed with message:
Failed while enforcing releasability. See above detailed error message.
```
Which version do you suggest we pick & how should we enforce this; dependencyManagement for `jakarta.activation:jakarta.activation-api:jar:1.2.2:runtime` ?
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on a diff in pull request #556: WICKET-7029 Add wicket-migration module
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #556:
URL: https://github.com/apache/wicket/pull/556#discussion_r1133607695
##########
pom.xml:
##########
@@ -119,7 +119,8 @@
<module>wicket-native-websocket</module>
<module>wicket-bean-validation</module>
<module>wicket-user-guide</module>
- </modules>
+ <module>wicket-migration</module>
Review Comment:
No need of renaming!
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] timtebeek commented on a diff in pull request #556: WICKET-7029 Add wicket-migration module
Posted by "timtebeek (via GitHub)" <gi...@apache.org>.
timtebeek commented on code in PR #556:
URL: https://github.com/apache/wicket/pull/556#discussion_r1133324315
##########
wicket-migration/src/test/java/org/apache/wicket/migration/MigrateToWicket10Test.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * Copyright (c) 2010-2023. wicket Framework
+ *
+ * Licensed 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.
+ */
+
+package org.apache.wicket.migration;
+
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.openrewrite.config.Environment;
+import org.openrewrite.java.JavaParser;
+import org.openrewrite.test.RecipeSpec;
+import org.openrewrite.test.RewriteTest;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.openrewrite.java.Assertions.java;
+import static org.openrewrite.java.Assertions.mavenProject;
+import static org.openrewrite.maven.Assertions.pomXml;
+
+class MigrateToWicket10Test implements RewriteTest {
+
+ @Override
+ public void defaults(RecipeSpec spec) {
+ spec
+ .parser(JavaParser.fromJavaVersion()
+ .logCompilationWarningsAndErrors(true)
+ .classpath("rewrite-migrate-java"))
+ .recipe(Environment.builder()
+ .scanRuntimeClasspath()
+ .build()
+ .activateRecipes("org.apache.wicket.MigrateToWicket10"));
+ }
+
+ @Test
+ void migrateImports() {
+ //language=java
+ rewriteRun(
+ java("""
+ package org.apache.wicket.http2.markup.head;
+
+ public class PushHeaderItem { }"""),
+ java(
+ """
+ package sample.wicket;
+
+ import org.apache.wicket.http2.markup.head.PushHeaderItem;
+
+ class ATest {
+ PushHeaderItem item;
+ }""",
+ """
+ package sample.wicket;
+
+ import org.apache.wicket.markup.head.http2.PushHeaderItem;
+
+ class ATest {
+ PushHeaderItem item;
+ }"""
+ ));
+ }
+
+ @Test
+ @Disabled("Required a first release of 10.x")
Review Comment:
This test resolves against Maven Central, and can be enabled as soon as a first 10.x is available there.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] timtebeek commented on pull request #556: WICKET-7029 Add wicket-migration module
Posted by "timtebeek (via GitHub)" <gi...@apache.org>.
timtebeek commented on PR #556:
URL: https://github.com/apache/wicket/pull/556#issuecomment-1481036389
Awesome, great to see this made it in! Happy to discuss whenever you need help for further migration steps. :)
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] timtebeek commented on a diff in pull request #556: WICKET-7029 Add wicket-migration module
Posted by "timtebeek (via GitHub)" <gi...@apache.org>.
timtebeek commented on code in PR #556:
URL: https://github.com/apache/wicket/pull/556#discussion_r1133573194
##########
pom.xml:
##########
@@ -119,7 +119,8 @@
<module>wicket-native-websocket</module>
<module>wicket-bean-validation</module>
<module>wicket-user-guide</module>
- </modules>
+ <module>wicket-migration</module>
Review Comment:
Open to adopt this as requested, although I'd though to sketch how we typically structure migrations. For comparison we also support [migrations through major versions of Spring](https://github.com/openrewrite/rewrite-spring/tree/main/src/main/resources/META-INF/rewrite), which are all in the same module as separate yaml files, which are chained together to support a migration from whatever version users are using.
If we were to adopt individual modules per major version, those modules would then likely have a dependency chain running through them, with likely no more than a yaml file and perhaps a few recipes per module. That could also be managed with a package within a single migration module.
Open to adopt the structure as suggested, but if you were to ask me such a distinction isn't necessary right now. So whatever you then think is best given the above, and your own preferences.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on pull request #556: WICKET-7029 Add wicket-migration module
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #556:
URL: https://github.com/apache/wicket/pull/556#issuecomment-1469817550
Please rebase to latest master. There was a commit earlier today that updated the dependencies.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] timtebeek commented on a diff in pull request #556: WICKET-7029 Add wicket-migration module
Posted by "timtebeek (via GitHub)" <gi...@apache.org>.
timtebeek commented on code in PR #556:
URL: https://github.com/apache/wicket/pull/556#discussion_r1133606848
##########
pom.xml:
##########
@@ -119,7 +119,8 @@
<module>wicket-native-websocket</module>
<module>wicket-bean-validation</module>
<module>wicket-user-guide</module>
- </modules>
+ <module>wicket-migration</module>
Review Comment:
Indeed! Hadn't documented that explicitly above, but that's how one can selectively update to an intermediate version if so desired. Would you then still want to introduce any naming or package changes before we resolve this conversation thread?
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g merged pull request #556: WICKET-7029 Add wicket-migration module
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g merged PR #556:
URL: https://github.com/apache/wicket/pull/556
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] timtebeek commented on a diff in pull request #556: WICKET-7029 Add wicket-migration module
Posted by "timtebeek (via GitHub)" <gi...@apache.org>.
timtebeek commented on code in PR #556:
URL: https://github.com/apache/wicket/pull/556#discussion_r1133324249
##########
wicket-migration/src/test/java/org/apache/wicket/migration/MigrateToWicket10Test.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * Copyright (c) 2010-2023. wicket Framework
+ *
+ * Licensed 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.
+ */
+
+package org.apache.wicket.migration;
+
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.openrewrite.config.Environment;
+import org.openrewrite.java.JavaParser;
+import org.openrewrite.test.RecipeSpec;
+import org.openrewrite.test.RewriteTest;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.openrewrite.java.Assertions.java;
+import static org.openrewrite.java.Assertions.mavenProject;
+import static org.openrewrite.maven.Assertions.pomXml;
+
+class MigrateToWicket10Test implements RewriteTest {
+
+ @Override
+ public void defaults(RecipeSpec spec) {
+ spec
+ .parser(JavaParser.fromJavaVersion()
+ .logCompilationWarningsAndErrors(true)
+ .classpath("rewrite-migrate-java"))
+ .recipe(Environment.builder()
+ .scanRuntimeClasspath()
+ .build()
+ .activateRecipes("org.apache.wicket.MigrateToWicket10"));
+ }
+
+ @Test
+ void migrateImports() {
+ //language=java
+ rewriteRun(
+ java("""
+ package org.apache.wicket.http2.markup.head;
+
+ public class PushHeaderItem { }"""),
Review Comment:
This is a quick stub to ensure the replacement below can access the type without dependency on wicket-http2-core:9.x
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] timtebeek commented on pull request #556: WICKET-7029 Add wicket-migration module
Posted by "timtebeek (via GitHub)" <gi...@apache.org>.
timtebeek commented on PR #556:
URL: https://github.com/apache/wicket/pull/556#issuecomment-1465303680
Couple comments:
- Wasn't sure if you'd already want to update the documentation at this stage, so the upgrade command is only listed above for now.
- The current migration steps could all be achieved with Yaml recipes as far as I could tell; hence no custom recipe implementations yet.
- The `BestPractices` recipe currently chains `MigrateToWicket10`, which seems sensible.
- Alternatively you can also already add the `BestPractices` recipe _only_ to the 9.x branch, and preemptively solve any deprecations there already.
- I'll have limited time to continue work on this, but hope it's a useful start for Apache Wicket to continue.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on a diff in pull request #556: WICKET-7029 Add wicket-migration module
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #556:
URL: https://github.com/apache/wicket/pull/556#discussion_r1133495627
##########
pom.xml:
##########
@@ -119,7 +119,8 @@
<module>wicket-native-websocket</module>
<module>wicket-bean-validation</module>
<module>wicket-user-guide</module>
- </modules>
+ <module>wicket-migration</module>
Review Comment:
I think it would be good to rename this module (and its Java package) to include `9_to_10`. This way it could stay for several major releases and it won't collide with the next migration, e.g. `10_to_11`
##########
pom.xml:
##########
@@ -119,7 +119,8 @@
<module>wicket-native-websocket</module>
<module>wicket-bean-validation</module>
<module>wicket-user-guide</module>
- </modules>
+ <module>wicket-migration</module>
Review Comment:
Or even better let's introduce an intermediate Maven module for `migration` and then all `x_to_y` modules will be children of it.
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [wicket] martin-g commented on pull request #556: WICKET-7029 Add wicket-migration module
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #556:
URL: https://github.com/apache/wicket/pull/556#issuecomment-1481037913
Added an item to the [Migration Page](https://cwiki.apache.org/confluence/display/WICKET/Migration+to+Wicket+10.0#MigrationtoWicket10.0-AddmigrationrecipestoWicket10WICKET-7029)
--
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: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org