Conversation
📝 WalkthroughWalkthroughA new documentation file is introduced to track unaddressed issues within the spring-migration/6 initiative. The document catalogs issues assigned to other contributors, identifies parent/tracking issues, and provides summary statistics on issue distribution and resolution status. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
7c4e12e to
902d760
Compare
This PR adds comprehensive rules to assist with migrating from Spring Framework 5.x to 6.0, addressing 11 open issues. ## New Rules Added ### Removed APIs (spring-framework-5.x-to-6.0-removed-apis.yaml) - 00010: RPC-style remoting removed (RMI, HTTP Invoker, Hessian, JAX-WS) - 00020: Dedicated EJB access removed ### Core Container (spring-framework-5.x-to-6.0-core-container.yaml) - 00040: javax.inject.Inject must migrate to jakarta.inject.Inject - 00050: LocalVariableTableParameterNameDiscoverer deprecated ### Data Access (spring-framework-5.x-to-6.0-data-access.yaml) - 00040: Deprecated exceptions (CannotSerializeTransactionException, DeadlockLoserDataAccessException) - 00050: JDBC exception translator default changed - 00060: Hibernate Validator 7.0+ required ### Web Applications (spring-framework-5.x-to-6.0-web-applications.yaml) - 00050: CommonsMultipartResolver removed - 00060: Apache Tiles integration removed - 00070: FreeMarker JSP support removed - 00080: Servlet mocks require Servlet API 6.0 - 00090: Web server upgrade required (Tomcat 10.1+, Jetty 11+, Undertow 2.2.19+) ### HttpClient (spring-framework-5.x-to-6.0-httpclient.yaml) - NEW FILE - 00010: Apache HttpClient 4.x code must migrate to 5.x - 00020: Apache HttpClient 4.x dependency detected ## Issues Addressed Fixes konveyor#134, konveyor#135, konveyor#137, konveyor#139, konveyor#146, konveyor#147, konveyor#149, konveyor#150, konveyor#155, konveyor#167, konveyor#175 ## Issues Not Addressed See SPRING_MIGRATION_UNADDRESSED.md for issues already assigned to other contributors or covered by existing rules. Signed-off-by: Dylan Murray <dymurray@redhat.com>
902d760 to
4eb6e2e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@SPRING_MIGRATION_UNADDRESSED.md`:
- Around line 29-34: Update the summary paragraph that currently reads "Issues
already assigned/covered: 11" to split it into two distinct counts (e.g.,
"Issues already assigned: N" and "Issues already covered: M") and adjust the
"Total issues with spring-migration/6" and "Issues addressed in this PR" numbers
as needed to reflect the new categorization; specifically locate the line
containing the literal text "Issues already assigned/covered: 11" and replace it
with separate labeled counts and ensure the overall totals are consistent with
those new values.
- Around line 5-22: Update the "## Already Assigned to Other Contributors"
section to separate truly assigned issues from those already covered: create two
subsections ("Issues assigned to other contributors" containing `#145` and `#152`
with their "In progress by assignee" notes, and "Issues already covered" listing
`#133`, `#136`, `#138`, `#144`, `#148`, `#151`, `#156`, `#166` with their "Already covered by
..." notes), update the table headers/rows accordingly and ensure the section
title and Notes column text accurately reflect which issues are in-progress vs.
already addressed.
| ## Already Assigned to Other Contributors | ||
|
|
||
| The following issues are already assigned to other contributors and were skipped to avoid duplication of effort: | ||
|
|
||
| | Issue | Title | Assignee | Notes | | ||
| |-------|-------|----------|-------| | ||
| | #133 | Create rule for minimum usage of Java17 and JakartaEE 9+ | jmle | Already covered by existing baseline rules | | ||
| | #136 | Create rules for removed `org.springframework.cache.ehcache` package | jmle | Already covered by removed-apis-00001 | | ||
| | #138 | Create rules for bean property determination | jmle | Already covered by core-container-00001 | | ||
| | #144 | Create rules for Hibernate Validator upgrade | jmle | Covered by new data-access-00060 rule | | ||
| | #145 | Create rules for EclipseLink | billwheatley | In progress by assignee | | ||
| | #148 | Create rules for backwards compatibility with database-specific error codes | jayfray12 | Already covered by data-access-00030 | | ||
| | #151 | Create rules for the trailing slash matching configuration | jmle | Already covered by web-applications-00001 | | ||
| | #152 | Create rules for controller detection | jmle | In progress by assignee | | ||
| | #154 | Create rules for RestTemplate Apache HttpClient 5 requirement | jmle | Related to #175 which we addressed | | ||
| | #156 | Create rules for `SourceHttpMessageConverter` | rromannissen | Already covered by web-applications-00030 | | ||
| | #166 | Create rules for migration to Hibernate 5.6 | jayfray12 | Already covered by data-access-00001 | | ||
|
|
There was a problem hiding this comment.
Clarify the distinction between "assigned" and "already covered" issues.
The section title indicates these issues are "Already Assigned to Other Contributors," but the majority of entries in the Notes column state "Already covered by [rule]" (e.g., #133, #136, #138, #144, #148, #151, #156, #166). This suggests these issues were actually addressed by existing or new rules in this PR, not that they're assigned to other contributors.
Only #145 and #152 have notes indicating "In progress by assignee," which aligns with the section title. Consider restructuring to separate:
- Issues assigned to others (truly in-progress elsewhere)
- Issues already covered (addressed by existing/new rules in this PR)
This distinction is important for maintainers to understand which issues still need work versus which are complete.
📋 Suggested restructure
-## Already Assigned to Other Contributors
+## Issues Assigned to Other Contributors
-The following issues are already assigned to other contributors and were skipped to avoid duplication of effort:
+The following issues are assigned to other contributors and were skipped to avoid duplication of effort:
| Issue | Title | Assignee | Notes |
|-------|-------|----------|-------|
-| `#133` | Create rule for minimum usage of Java17 and JakartaEE 9+ | jmle | Already covered by existing baseline rules |
-| `#136` | Create rules for removed `org.springframework.cache.ehcache` package | jmle | Already covered by removed-apis-00001 |
-| `#138` | Create rules for bean property determination | jmle | Already covered by core-container-00001 |
-| `#144` | Create rules for Hibernate Validator upgrade | jmle | Covered by new data-access-00060 rule |
| `#145` | Create rules for EclipseLink | billwheatley | In progress by assignee |
-| `#148` | Create rules for backwards compatibility with database-specific error codes | jayfray12 | Already covered by data-access-00030 |
-| `#151` | Create rules for the trailing slash matching configuration | jmle | Already covered by web-applications-00001 |
| `#152` | Create rules for controller detection | jmle | In progress by assignee |
-| `#154` | Create rules for RestTemplate Apache HttpClient 5 requirement | jmle | Related to `#175` which we addressed |
-| `#156` | Create rules for `SourceHttpMessageConverter` | rromannissen | Already covered by web-applications-00030 |
-| `#166` | Create rules for migration to Hibernate 5.6 | jayfray12 | Already covered by data-access-00001 |
+
+## Issues Already Covered by Existing or New Rules
+
+The following issues are already addressed by rules in this PR or existing rulesets:
+
+| Issue | Title | Covered By |
+|-------|-------|------------|
+| `#133` | Create rule for minimum usage of Java17 and JakartaEE 9+ | Existing baseline rules |
+| `#136` | Create rules for removed `org.springframework.cache.ehcache` package | removed-apis-00001 |
+| `#138` | Create rules for bean property determination | core-container-00001 |
+| `#144` | Create rules for Hibernate Validator upgrade | data-access-00060 (new) |
+| `#148` | Create rules for backwards compatibility with database-specific error codes | data-access-00030 |
+| `#151` | Create rules for the trailing slash matching configuration | web-applications-00001 |
+| `#154` | Create rules for RestTemplate Apache HttpClient 5 requirement | Related to `#175` |
+| `#156` | Create rules for `SourceHttpMessageConverter` | web-applications-00030 |
+| `#166` | Create rules for migration to Hibernate 5.6 | data-access-00001 |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Already Assigned to Other Contributors | |
| The following issues are already assigned to other contributors and were skipped to avoid duplication of effort: | |
| | Issue | Title | Assignee | Notes | | |
| |-------|-------|----------|-------| | |
| | #133 | Create rule for minimum usage of Java17 and JakartaEE 9+ | jmle | Already covered by existing baseline rules | | |
| | #136 | Create rules for removed `org.springframework.cache.ehcache` package | jmle | Already covered by removed-apis-00001 | | |
| | #138 | Create rules for bean property determination | jmle | Already covered by core-container-00001 | | |
| | #144 | Create rules for Hibernate Validator upgrade | jmle | Covered by new data-access-00060 rule | | |
| | #145 | Create rules for EclipseLink | billwheatley | In progress by assignee | | |
| | #148 | Create rules for backwards compatibility with database-specific error codes | jayfray12 | Already covered by data-access-00030 | | |
| | #151 | Create rules for the trailing slash matching configuration | jmle | Already covered by web-applications-00001 | | |
| | #152 | Create rules for controller detection | jmle | In progress by assignee | | |
| | #154 | Create rules for RestTemplate Apache HttpClient 5 requirement | jmle | Related to #175 which we addressed | | |
| | #156 | Create rules for `SourceHttpMessageConverter` | rromannissen | Already covered by web-applications-00030 | | |
| | #166 | Create rules for migration to Hibernate 5.6 | jayfray12 | Already covered by data-access-00001 | | |
| ## Issues Assigned to Other Contributors | |
| The following issues are assigned to other contributors and were skipped to avoid duplication of effort: | |
| | Issue | Title | Assignee | Notes | | |
| |-------|-------|----------|-------| | |
| | `#145` | Create rules for EclipseLink | billwheatley | In progress by assignee | | |
| | `#152` | Create rules for controller detection | jmle | In progress by assignee | | |
| ## Issues Already Covered by Existing or New Rules | |
| The following issues are already addressed by rules in this PR or existing rulesets: | |
| | Issue | Title | Covered By | | |
| |-------|-------|------------| | |
| | `#133` | Create rule for minimum usage of Java17 and JakartaEE 9+ | Existing baseline rules | | |
| | `#136` | Create rules for removed `org.springframework.cache.ehcache` package | removed-apis-00001 | | |
| | `#138` | Create rules for bean property determination | core-container-00001 | | |
| | `#144` | Create rules for Hibernate Validator upgrade | data-access-00060 (new) | | |
| | `#148` | Create rules for backwards compatibility with database-specific error codes | data-access-00030 | | |
| | `#151` | Create rules for the trailing slash matching configuration | web-applications-00001 | | |
| | `#154` | Create rules for RestTemplate Apache HttpClient 5 requirement | Related to `#175` | | |
| | `#156` | Create rules for `SourceHttpMessageConverter` | web-applications-00030 | | |
| | `#166` | Create rules for migration to Hibernate 5.6 | data-access-00001 | |
🤖 Prompt for AI Agents
In `@SPRING_MIGRATION_UNADDRESSED.md` around lines 5 - 22, Update the "## Already
Assigned to Other Contributors" section to separate truly assigned issues from
those already covered: create two subsections ("Issues assigned to other
contributors" containing `#145` and `#152` with their "In progress by assignee"
notes, and "Issues already covered" listing `#133`, `#136`, `#138`, `#144`, `#148`, `#151`,
`#156`, `#166` with their "Already covered by ..." notes), update the table
headers/rows accordingly and ensure the section title and Notes column text
accurately reflect which issues are in-progress vs. already addressed.
| ## Summary | ||
|
|
||
| - **Total issues with spring-migration/6 label:** 23 | ||
| - **Issues addressed in this PR:** 11 | ||
| - **Issues already assigned/covered:** 11 | ||
| - **Parent/tracking issues:** 1 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Update summary to reflect distinct categories.
The summary line "Issues already assigned/covered: 11" combines two different categories into one count. If the table sections are restructured as suggested above, update this summary to reflect the distinct categories:
📊 Suggested summary update
## Summary
- **Total issues with spring-migration/6 label:** 23
- **Issues addressed in this PR:** 11
-- **Issues already assigned/covered:** 11
+- **Issues assigned to other contributors:** 2
+- **Issues already covered by existing or new rules:** 9
- **Parent/tracking issues:** 1(Adjust counts based on actual categorization after restructuring)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Summary | |
| - **Total issues with spring-migration/6 label:** 23 | |
| - **Issues addressed in this PR:** 11 | |
| - **Issues already assigned/covered:** 11 | |
| - **Parent/tracking issues:** 1 | |
| ## Summary | |
| - **Total issues with spring-migration/6 label:** 23 | |
| - **Issues addressed in this PR:** 11 | |
| - **Issues assigned to other contributors:** 2 | |
| - **Issues already covered by existing or new rules:** 9 | |
| - **Parent/tracking issues:** 1 |
🤖 Prompt for AI Agents
In `@SPRING_MIGRATION_UNADDRESSED.md` around lines 29 - 34, Update the summary
paragraph that currently reads "Issues already assigned/covered: 11" to split it
into two distinct counts (e.g., "Issues already assigned: N" and "Issues already
covered: M") and adjust the "Total issues with spring-migration/6" and "Issues
addressed in this PR" numbers as needed to reflect the new categorization;
specifically locate the line containing the literal text "Issues already
assigned/covered: 11" and replace it with separate labeled counts and ensure the
overall totals are consistent with those new values.
| description: JSR-330 @Inject annotation must be migrated to Jakarta namespace | ||
| message: | | ||
| The JSR-330 based `@Inject` annotation is to be found in `jakarta.inject` now. The `javax.inject` package | ||
| is no longer supported in Spring Framework 6.0 as part of the Jakarta EE 9+ baseline migration. |
There was a problem hiding this comment.
This one is correct, but I think we're missing the following from the guide (linked below):
The corresponding JSR-250 based annotations
@PostConstructand@PreDestroyare to be found in jakarta.annotation
| ``` | ||
| links: | ||
| - title: 'Spring 6.0 migration guide - Data Access' | ||
| url: https://github.com/spring-projects/spring-framework/wiki/Upgrading-to-Spring-Framework-6.x#data-access-and-transactions-1 |
There was a problem hiding this comment.
This link is not completely correct
| - title: 'Hibernate Validator 7.0 Release Announcement' | ||
| url: https://in.relation.to/2020/12/08/hibernate-validator-700-62-cr1-released/ | ||
| - title: 'Hibernate Validator Migration Guide' | ||
| url: https://hibernate.org/validator/documentation/migration-guide/ |
There was a problem hiding this comment.
We're missing here the link for the guide: https://github.com/spring-projects/spring-framework/wiki/Spring-Framework-6.0-Release-Notes#data-access-and-transactions
| pattern: org.apache.http.impl.client.HttpClientBuilder | ||
| - java.referenced: | ||
| pattern: org.apache.http.client.methods* | ||
| description: Apache HttpClient 4.x must be migrated to HttpClient 5.x for Spring 6 |
There was a problem hiding this comment.
I'm not completely sure where this comes from, it's not in the guide. I think we can remove this rule, I've done some checking and it doesn't seem to be necessary to make this migration.
There was a problem hiding this comment.
Well, it seems to be a bit more nuanced than that. According to the guide:
RestTemplate, or rather the HttpComponentsClientHttpRequestFactory, now requires Apache HttpClient 5.
I think we can change this rule to better frame this
There was a problem hiding this comment.
In fact I just saw that I had an open PR with a rule for this matter: #174
We can probably just use that one instead
| description: Apache HttpClient 4.x dependency detected - upgrade required for Spring 6 | ||
| message: | | ||
| Your project uses Apache HttpClient 4.x which is not compatible with Spring Framework 6.0. | ||
| Spring 6.0's `RestTemplate` with Apache HttpClient support requires HttpClient 5.x. |
There was a problem hiding this comment.
This message is better than the one from the rule above. We probably can merge these two into a single one to better convey what the guide says.
| - title: 'Spring 6.0 migration guide - Removed APIs' | ||
| url: https://github.com/spring-projects/spring-framework/wiki/Spring-Framework-6.0-Release-Notes#removed-apis | ||
| - title: 'Spring Issue #27422' | ||
| url: https://github.com/spring-projects/spring-framework/issues/27422 |
There was a problem hiding this comment.
I think we should remove this link
| when: | ||
| or: | ||
| - java.referenced: | ||
| pattern: org.springframework.web.multipart.commons.CommonsMultipartResolver |
There was a problem hiding this comment.
In the guide I see that this has been removed, but it doesn't say anything about the two below, can you double check?
| upperbound: 10.99.99 | ||
| - java.dependency: | ||
| name: io.undertow.undertow-core | ||
| upperbound: 2.2.18.Final |
There was a problem hiding this comment.
In the guide, it is stated that Spring 6.0 is
Compatible with latest web servers: Tomcat 10.1, Jetty 11, Undertow 2.3.
so I'm wondering if this rule is necessary. I would probably argue to remove it
Issues Addressed (11)
Fixes #134 , #135, #137, #139, #146, #147, #149, #150, #155, #167, #175