-
Notifications
You must be signed in to change notification settings - Fork 2
build!: remove compatibility with Vaadin 23 AND feat!: replace classNameGenerator with partNameGenerator AND deprecate: deprecate footer/header styles #153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughRefactors GridHelper to use part-name generators (renaming classes/methods), adds deprecated header/footer style accessors, updates CSS selector to part-based targeting, upgrades Vaadin/Java/Jetty and Maven profiles in pom.xml, and adds/adjusts GitHub Actions workflows for multiple Java/Vaadin combinations. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelper.java (1)
163-183: Javadoc still references "CSS class names" instead of "part names".The method has been renamed to
setPartNameGeneratorand usesColumn#setPartNameGeneratorin the@seetag, but lines 164-167 still describe "CSS class names". This creates an inconsistency with the API rename.Consider updating the Javadoc to align with the part-name terminology:
/** - * Sets the function that is used for generating CSS class names for all the cells in the rows in - * this grid. Returning {@code null} from the generator results in no custom class name being set. - * Multiple class names can be returned from the generator as space-separated. + * Sets the function that is used for generating part names for all the cells in the rows in + * this grid. Returning {@code null} from the generator results in no custom part name being set. + * Multiple part names can be returned from the generator as space-separated. * * <p> * If {@link Column#setPartNameGenerator(SerializableFunction)} is used together with this method, * resulting part names from both methods will be effective. * * @param partNameGenerator the part name generator to set. * @see Column#setPartNameGenerator(SerializableFunction) */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/maven-v1.yml(1 hunks).github/workflows/maven.yml(1 hunks)pom.xml(4 hunks)src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelper.java(3 hunks)src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelperPartNameGenerator.java(2 hunks)src/main/java/com/flowingcode/vaadin/addons/gridhelpers/SelectionFilterHelper.java(2 hunks)src/main/resources/META-INF/frontend/fcGridHelper/vaadin-grid.css(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-15T19:46:52.496Z
Learnt from: javier-godoy
Repo: FlowingCode/GridHelpers PR: 137
File: src/main/java/com/flowingcode/vaadin/addons/gridhelpers/HeaderFooterStylesHelper.java:145-161
Timestamp: 2025-09-15T19:46:52.496Z
Learning: In HeaderFooterStylesHelper.java, the getColumn() method using reflection on AbstractCell_getColumn.invoke(cell) should not return null when the reflection succeeds - it always returns a valid column object.
Applied to files:
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelper.java
🪛 actionlint (1.7.9)
.github/workflows/maven.yml
23-23: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
36-36: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/maven-v1.yml
23-23: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
36-36: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 GitHub Actions: Java CI with Maven
pom.xml
[error] 1-1: Command failed: mvn -B package --file pom.xml. Fatal error compiling: error: invalid target release: 18 (Maven Compiler Plugin 3.13.0) – ensure the Java compiler target matches the installed JDK (currently incompatible with target 18).
🔇 Additional comments (8)
src/main/resources/META-INF/frontend/fcGridHelper/vaadin-grid.css (1)
53-53: CSS selector correctly migrated to part-based naming.The change from
.fcGh-noselect(class selector) to[part~="fcGh-noselect"](part attribute selector) aligns with the migration from classNameGenerator to partNameGenerator in Vaadin 25. This matches the pattern used throughout the rest of the file and is the correct Web Component approach for styling parts. The Java implementation inSelectionFilterHelper.javacorrectly usessetHelperPartNameGenerator()to set thefcGh-noselectpart name when the filter predicate applies..github/workflows/maven-v1.yml (1)
23-23: Update outdated GitHub Actions version.The runner of
actions/setup-java@v3is too old to run on GitHub Actions. Update tov4or later for compatibility with current GitHub Actions runners.- name: Set up JDK uses: actions/setup-java@v3 with:Apply this to both jobs (build-vaadin23 and build-vaadin24):
- - uses: actions/setup-java@v3 + - uses: actions/setup-java@v4 with: java-version: '11' distribution: 'temurin' cache: mavenAlso applies to: 36-36
.github/workflows/maven.yml (2)
23-23: Update outdated GitHub Actions version.The runner of
actions/setup-java@v3is too old to run on GitHub Actions. Update tov4or later for compatibility with current GitHub Actions runners.Apply this to both jobs (build-vaadin24 and build-vaadin25):
- name: Set up JDK - uses: actions/setup-java@v3 + uses: actions/setup-java@v4 with: java-version: '17' distribution: 'temurin' cache: mavenAlso applies to: 36-36
29-29: Clarify Maven profile for build-vaadin24 job.The build-vaadin24 job runs without an explicit Maven profile (line 29), which should resolve to the base Vaadin 24 configuration from pom.xml. Verify that this is intentional and that the profile defaults are correct, especially given that the build-vaadin25 job explicitly specifies
-Pv25.src/main/java/com/flowingcode/vaadin/addons/gridhelpers/SelectionFilterHelper.java (1)
62-67: LGTM!The API rename from
setHelperClassNameGeneratortosetHelperPartNameGeneratoris correctly applied. The logic for applying the"fcGh-noselect"part name to non-selectable rows and clearing the generator when the predicate is null remains intact.src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelperPartNameGenerator.java (1)
34-58: LGTM!The class and field renames from class-name to part-name terminology are consistent and complete. The generator aggregation logic in
generators()properly combines the grid-level generator with helper-specific generators, and theapply()method correctly handles null values, deduplication, and space-separated part names.src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelper.java (2)
96-105: LGTM!The constructor correctly initializes the
GridHelperPartNameGeneratorand wires it to the grid. The call tosetPartNameGenerator(grid.getPartNameGenerator())properly preserves any existing generator while wrapping it with the helper's aggregation logic.
436-486: LGTM!The deprecated header/footer style methods are properly annotated with
@Deprecatedand include clear Javadoc guidance pointing users to the Vaadin 24/25 native APIs (HeaderCell#setPartName,FooterCell#setPartName, etc.). The deprecation aligns with the PR's objective to leverage built-in Vaadin header/footer styling support.
c9ea94f to
3097dbe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pom.xml (1)
567-595: Vaadin 25.0.0-rc1 is not available in the vaadin-prerelease repository.The version exists in Maven Central but the pom.xml configuration references
https://maven.vaadin.com/vaadin-prereleaseswhich does not contain this artifact. The build will fail during dependency resolution. Either remove the vaadin-prerelease repository configuration and rely on Maven Central, or update the Vaadin version to one actually available in that prerelease repository.
🧹 Nitpick comments (2)
.github/workflows/maven.yml (1)
23-23: Consider upgrading actions/setup-java to v4.The
actions/setup-java@v3action is deprecated and should be upgraded to v4 for continued compatibility with GitHub Actions runners.Apply this diff:
- uses: actions/setup-java@v3 + uses: actions/setup-java@v4Apply the same change on line 36.
Also applies to: 36-36
.github/workflows/maven-v1.yml (1)
23-23: Consider upgrading actions/setup-java to v4.The
actions/setup-java@v3action is deprecated. Consider upgrading to v4 for continued compatibility.Apply this diff on both lines 23 and 36:
- uses: actions/setup-java@v3 + uses: actions/setup-java@v4Also applies to: 36-36
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/maven-v1.yml(1 hunks).github/workflows/maven.yml(1 hunks)pom.xml(4 hunks)src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelper.java(3 hunks)src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelperPartNameGenerator.java(2 hunks)src/main/java/com/flowingcode/vaadin/addons/gridhelpers/SelectionFilterHelper.java(2 hunks)src/main/resources/META-INF/frontend/fcGridHelper/vaadin-grid.css(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-15T19:46:52.496Z
Learnt from: javier-godoy
Repo: FlowingCode/GridHelpers PR: 137
File: src/main/java/com/flowingcode/vaadin/addons/gridhelpers/HeaderFooterStylesHelper.java:145-161
Timestamp: 2025-09-15T19:46:52.496Z
Learning: In HeaderFooterStylesHelper.java, the getColumn() method using reflection on AbstractCell_getColumn.invoke(cell) should not return null when the reflection succeeds - it always returns a valid column object.
Applied to files:
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelper.java
🪛 actionlint (1.7.9)
.github/workflows/maven.yml
23-23: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
36-36: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/maven-v1.yml
23-23: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
36-36: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (14)
src/main/resources/META-INF/frontend/fcGridHelper/vaadin-grid.css (1)
53-53: LGTM! CSS selector correctly updated for part-based styling.The selector change from
.fcGh-noselectto[part~="fcGh-noselect"]correctly implements the transition from class-name-based to part-name-based styling, aligning with Vaadin's part attribute approach and the broader API refactor in this PR..github/workflows/maven.yml (2)
18-29: LGTM! Workflow correctly updated for Vaadin 24 baseline.The job rename and Java 17 upgrade align with the Vaadin 24 baseline (24.8.14) defined in pom.xml. The removal of the
-Pv23flag is correct since the default profile now targets Vaadin 24.
31-42: LGTM! Workflow correctly configured for Vaadin 25 testing.The job configuration appropriately uses Java 21 and the
-Pv25profile to test against Vaadin 25.0.0-rc1, consistent with the v25 profile in pom.xml.pom.xml (3)
13-15: LGTM! Java and Vaadin versions correctly configured.The upgrade to Vaadin 24.8.14 baseline with Java 17 source/target is appropriate. Both compiler source and target are correctly set to 17, matching the build-vaadin24 job in maven.yml.
19-22: LGTM! Jetty and plugin versions appropriately updated.Jetty 11.0.26 is required for Java 17+ compatibility (Jetty 9.4.x only supports Java 11 max). The driver plugin bump and frontend.hotdeploy flag are reasonable development configuration updates.
66-66: commons-demo version 4.3.0 is available in Maven Central and will be successfully resolved..github/workflows/maven-v1.yml (1)
18-29: LGTM! Workflow correctly configured for 1.x branch maintenance.The workflow appropriately maintains Vaadin 23 and 24 build compatibility for the 1.x maintenance branch, with correct Java versions (11 for v23, 17 for v24) and Maven profiles.
Also applies to: 31-42
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelper.java (4)
86-92: LGTM! Field and method correctly renamed for part-based API.The rename from
helperClassNameGeneratortohelperPartNameGeneratorand the corresponding method update correctly implement the transition to Vaadin's part-name-based styling API.
98-99: LGTM! Constructor correctly initializes part-name generator.The initialization uses
GridHelperPartNameGeneratorand delegates tosetPartNameGenerator, correctly implementing the new part-based API.
436-486: LGTM! Deprecated methods provide backwards compatibility.The four deprecated header/footer style accessor methods appropriately:
- Maintain backwards compatibility for existing code
- Provide clear deprecation guidance pointing to
HeaderCell.setPartName()andFooterCell.getPartName()- Delegate to the existing
headerFooterStylesHelperfor implementationThe deprecation aligns with Vaadin 24-25 providing native header/footer styling support as mentioned in the PR description.
163-183: The implementation correctly uses Vaadin 25'ssetPartNameGeneratorAPI. The method properly:
- Wraps the grid with
helperPartNameGenerator(a decorator that handles GridHelper's own generators)- Handles both
GridHelperPartNameGeneratorinstances and generic functions appropriately- Delegates to the grid's native
setPartNameGeneratormethodVaadin 25 confirmed to have removed
setClassNameGeneratorand replaced it with the part-name API (setPartNameGenerator), which uses CSS::part()selectors for styling.src/main/java/com/flowingcode/vaadin/addons/gridhelpers/SelectionFilterHelper.java (1)
62-66: LGTM! Method call correctly updated for renamed API.The calls to
setHelperPartNameGenerator(both for non-null and null predicate cases) correctly update to the renamed API while preserving the existing behavior of generating "fcGh-noselect" part names for unselectable rows.src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelperPartNameGenerator.java (2)
34-51: LGTM! Class and API correctly renamed for part-based terminology.The systematic rename from
GridHelperClassNameGeneratortoGridHelperPartNameGeneratorand the corresponding field/method updates correctly implement the shift to part-based naming while preserving all existing logic for aggregating and managing part name generators.
53-79: LGTM! Generator aggregation logic preserved correctly.The
generators()andapply()methods correctly use the renamed fields (gridPartNameGeneratorandhelperPartNameGenerators) while maintaining the existing behavior of aggregating, trimming, deduplicating, and joining part names with space separation.
mlopezFC
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the following in the description of the PR:
Marked current Footer and Header styling methods as deprecated, since Vaadin 24 and 25 already provide support for styling headers.
But also according to the PR, is is dropping the support for V23, so it doesn't make sense to deprecate something that cannot be used ... unless the description is wrong ...
I try to avoid removing features that have not being deprecated in an earlier version. In this case, I think that deprecation makes easier to migrate from version 1.x to version 2.x of the add-on (since the feature still works in Vaadin 25). We could not deprecate in version 1.x because there was no workaround in Vaadin 23. |
d0fd88b to
26017f3
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelper.java (1)
436-486: LGTM: Deprecated methods properly documented with clear migration path.The four deprecated header/footer styling methods are well-implemented:
- Each has clear Javadoc explaining the deprecation rationale
- Migration guidance points users to the native Vaadin 24+ API (
HeaderCell#setPartName/FooterCell#setPartName)- Existing functionality is preserved through delegation to
headerFooterStylesHelper- Consistent pattern across all four methods
This aligns with the PR objective of deprecating these methods since Vaadin 24–25 now provide native header/footer styling support.
Optional: Consider adding the target removal version to the
@Deprecatedannotation (e.g.,@Deprecated(forRemoval = true, since = "2.0")) to help users plan their migration timeline.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/maven-v1.yml(1 hunks).github/workflows/maven.yml(1 hunks)pom.xml(3 hunks)src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelper.java(3 hunks)src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelperPartNameGenerator.java(2 hunks)src/main/java/com/flowingcode/vaadin/addons/gridhelpers/SelectionFilterHelper.java(2 hunks)src/main/resources/META-INF/frontend/fcGridHelper/vaadin-grid.css(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/resources/META-INF/frontend/fcGridHelper/vaadin-grid.css
- src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelperPartNameGenerator.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-15T19:46:52.496Z
Learnt from: javier-godoy
Repo: FlowingCode/GridHelpers PR: 137
File: src/main/java/com/flowingcode/vaadin/addons/gridhelpers/HeaderFooterStylesHelper.java:145-161
Timestamp: 2025-09-15T19:46:52.496Z
Learning: In HeaderFooterStylesHelper.java, the getColumn() method using reflection on AbstractCell_getColumn.invoke(cell) should not return null when the reflection succeeds - it always returns a valid column object.
Applied to files:
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelper.java
🪛 actionlint (1.7.9)
.github/workflows/maven.yml
23-23: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
36-36: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/maven-v1.yml
23-23: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
36-36: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (9)
src/main/java/com/flowingcode/vaadin/addons/gridhelpers/SelectionFilterHelper.java (1)
62-66: LGTM: API migration to partNameGenerator is correct.The method calls have been correctly updated to use
setHelperPartNameGeneratorinstead ofsetHelperClassNameGenerator, aligning with Vaadin 25's removal of the classNameGenerator API. The logic remains unchanged—just the API surface was updated.src/main/java/com/flowingcode/vaadin/addons/gridhelpers/GridHelper.java (3)
86-92: LGTM: Field and method rename aligns with Vaadin 25 API.The field type and method name have been correctly updated from
ClassNameGeneratortoPartNameGeneratorterminology. The internal logic remains unchanged, maintaining behavioral consistency.
96-105: LGTM: Constructor correctly initializes part name generator.The constructor properly instantiates the
GridHelperPartNameGeneratorand preserves any existing part name generator from the grid by passing it tosetPartNameGenerator. The null case (if the grid has no existing generator) is correctly handled in thesetPartNameGeneratormethod at line 181.
163-183: LGTM: Part name generator delegation logic is sound.The method correctly:
- Updates documentation to reference "part names" instead of "class names"
- Wraps the grid's part name generator with the helper's generator (line 176) to enable additional functionality
- Avoids double-wrapping by detecting if the incoming generator is already a
GridHelperPartNameGenerator(lines 177-179)- Preserves custom user generators by storing them internally (line 181)
This delegation pattern allows the helper to augment the grid's part name generation without breaking existing functionality.
.github/workflows/maven-v1.yml (1)
11-15: LGTM!The workflow correctly targets the "1.x" branch, enabling separate CI for legacy Vaadin 23/24 support while the master branch moves to Vaadin 24/25.
pom.xml (4)
19-19: LGTM!The Jetty 11 upgrade is required for Java 17+ compatibility due to the Jakarta EE namespace migration. This is appropriate for the development/test environment.
21-22: LGTM!The driver-binary-downloader version bump and frontend.hotdeploy property addition are appropriate improvements for development workflow.
66-66: LGTM!The commons-demo upgrade to 4.3.0 is appropriate for Vaadin 24 compatibility in the test/demo scope.
13-15: Core upgrades align with PR objectives.Vaadin 24 is the recommended version with Java 17 support, and the Vaadin 24.8.14 and Java 17 upgrades correctly implement the removal of Vaadin 23 compatibility. Vaadin 24 requires Java 17 or later, making these appropriate breaking changes for the 2.x release. Vaadin 24.9 is the last scheduled 24.x release, and 24.8.14 remains a stable version within the 24.8 maintenance series.
| build-vaadin23: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Set up JDK | ||
| uses: actions/setup-java@v3 | ||
| with: | ||
| java-version: '11' | ||
| distribution: 'temurin' | ||
| cache: maven | ||
| - name: Build (Vaadin 23) | ||
| run: mvn -B package --file pom.xml -Pv23 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Upgrade to actions/setup-java@v4.
The actions/setup-java@v3 action is deprecated. Update to v4 to ensure compatibility with current GitHub Actions runners.
Apply this diff:
- name: Set up JDK
- uses: actions/setup-java@v3
+ uses: actions/setup-java@v4
with:
java-version: '11'
distribution: 'temurin'
cache: maven📝 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.
| build-vaadin23: | |
| runs-on: ubuntu-latest | |
| steps: | |
| - uses: actions/checkout@v4 | |
| - name: Set up JDK | |
| uses: actions/setup-java@v3 | |
| with: | |
| java-version: '11' | |
| distribution: 'temurin' | |
| cache: maven | |
| - name: Build (Vaadin 23) | |
| run: mvn -B package --file pom.xml -Pv23 | |
| build-vaadin23: | |
| runs-on: ubuntu-latest | |
| steps: | |
| - uses: actions/checkout@v4 | |
| - name: Set up JDK | |
| uses: actions/setup-java@v4 | |
| with: | |
| java-version: '11' | |
| distribution: 'temurin' | |
| cache: maven | |
| - name: Build (Vaadin 23) | |
| run: mvn -B package --file pom.xml -Pv23 |
🧰 Tools
🪛 actionlint (1.7.9)
23-23: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
.github/workflows/maven-v1.yml lines 18-29: the workflow uses the deprecated
actions/setup-java@v3; update the step to use actions/setup-java@v4, keeping the
same with inputs (java-version: '11', distribution: 'temurin', cache: maven) and
ensure the step name remains "Set up JDK" so the runner uses the supported v4
action.
| build-vaadin25: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Set up JDK | ||
| uses: actions/setup-java@v3 | ||
| with: | ||
| java-version: '17' | ||
| java-version: '21' | ||
| distribution: 'temurin' | ||
| cache: maven | ||
| - name: Build (Vaadin 24) | ||
| run: mvn -B package --file pom.xml -Pv24 | ||
| - name: Build (Vaadin 25) | ||
| run: mvn -B package --file pom.xml -Pv25 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Vaadin 25 profile correctly configured.
The job correctly updates to Java 21 and activates the v25 profile, enabling testing against Vaadin 25 RC.
However, update to actions/setup-java@v4 to avoid deprecation:
- name: Set up JDK
- uses: actions/setup-java@v3
+ uses: actions/setup-java@v4
with:
java-version: '21'
distribution: 'temurin'
cache: maven📝 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.
| build-vaadin25: | |
| runs-on: ubuntu-latest | |
| steps: | |
| - uses: actions/checkout@v4 | |
| - name: Set up JDK | |
| uses: actions/setup-java@v3 | |
| with: | |
| java-version: '17' | |
| java-version: '21' | |
| distribution: 'temurin' | |
| cache: maven | |
| - name: Build (Vaadin 24) | |
| run: mvn -B package --file pom.xml -Pv24 | |
| - name: Build (Vaadin 25) | |
| run: mvn -B package --file pom.xml -Pv25 | |
| build-vaadin25: | |
| runs-on: ubuntu-latest | |
| steps: | |
| - uses: actions/checkout@v4 | |
| - name: Set up JDK | |
| uses: actions/setup-java@v4 | |
| with: | |
| java-version: '21' | |
| distribution: 'temurin' | |
| cache: maven | |
| - name: Build (Vaadin 25) | |
| run: mvn -B package --file pom.xml -Pv25 |
🧰 Tools
🪛 actionlint (1.7.9)
36-36: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
.github/workflows/maven.yml lines 31-42: the workflow uses actions/setup-java@v3
which is deprecated; update the action reference to actions/setup-java@v4 in the
"Set up JDK" step and keep the existing with: parameters (java-version: '21',
distribution: 'temurin', cache: maven); ensure the uses line is changed only to
@v4 and commit the updated workflow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coderabbitai Please create issue about it and mention that the change must be applied across all repos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Create GitHub issue about updating actions/setup-java to v4 across all repositories
gh issue create \
--title "Update actions/setup-java from v3 to v4 across all repositories" \
--body "## Description
The GitHub Actions workflow is currently using \`actions/setup-java@v3\`, which is deprecated. This action should be updated to \`actions/setup-java@v4\` to avoid deprecation warnings.
## Scope
This change needs to be applied across **all repositories** that use GitHub Actions workflows with the \`actions/setup-java\` action.
## Context
This issue was identified during the review of PR #153.
- PR: https://github.com/FlowingCode/GridHelpers/pull/153
- Comment: https://github.com/FlowingCode/GridHelpers/pull/153#discussion_r2623351457
## Action Items
1. Identify all repositories with GitHub Actions workflows using \`actions/setup-java@v3\`
2. Update the action reference to \`actions/setup-java@v4\` in each repository
3. Verify that existing \`with:\` parameters remain compatible
Requested by: @javier-godoy" \
--assignee javier-godoy
Length of output: 53
@javier-godoy, I've created issue #154 about updating actions/setup-java from v3 to v4. The issue mentions that this change needs to be applied across all repositories and includes backlinks to this PR and comment for reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue has been transferred to https://github.com/FlowingCode/AddonsInternal/issues/134
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@javier-godoy, thank you for the update! I've noted that the issue has been transferred to the AddonsInternal repository. The new tracking link is:
| <properties> | ||
| <maven.compiler.source>21</maven.compiler.source> | ||
| <maven.compiler.target>21</maven.compiler.target> | ||
| <vaadin.version>25.0.0-beta7</vaadin.version> | ||
| <vaadin.version>25.0.0-rc1</vaadin.version> | ||
| <jetty.version>11.0.26</jetty.version> | ||
| </properties> | ||
| <repositories> | ||
| <repository> | ||
| <id>vaadin-prerelease</id> | ||
| <url>https://maven.vaadin.com/vaadin-prereleases</url> | ||
| </repository> | ||
| </repositories> | ||
| <pluginRepositories> | ||
| <pluginRepository> | ||
| <id>vaadin-prerelease</id> | ||
| <url>https://maven.vaadin.com/vaadin-prereleases</url> | ||
| </pluginRepository> | ||
| </pluginRepositories> | ||
| <dependencies> | ||
| <dependency> | ||
| <groupId>com.flowingcode.vaadin.addons.demo</groupId> | ||
| <artifactId>commons-demo</artifactId> | ||
| <version>5.0.0</version> | ||
| <scope>test</scope> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>com.vaadin</groupId> | ||
| <artifactId>vaadin-dev</artifactId> | ||
| </dependency> | ||
| </dependencies> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vaadin 25 profile uses an outdated release candidate.
The v25 profile specifies 25.0.0-rc1, but Vaadin 25.0.0-rc2 is available. Consider upgrading to rc2 for the latest pre-release improvements. The vaadin-dev dependency addition and Java 21 requirement are appropriate for Vaadin 25.
🤖 Prompt for AI Agents
In pom.xml around lines 536 to 553, the vaadin.version property is pinned to
25.0.0-rc1 which is outdated; update the <vaadin.version> value to 25.0.0-rc2
(or the latest available rc) in the properties section, save the file, then run
a quick mvn -DskipTests package or mvn dependency:resolve to ensure all Vaadin
artifacts (including vaadin-dev) resolve cleanly and adjust any related
plugin/configuration if incompatible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be handled in a separate task
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@javier-godoy, understood! I've noted that the Vaadin version update will be addressed separately.
mlopezFC
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM



classNameGeneratorand introducedpartNameGeneratoras replacement (sinceclassNameGeneratorwas removed in Vaadin 25).Summary by CodeRabbit
New Features
Bug Fixes
Chores
Breaking Changes
✏️ Tip: You can customize this high-level summary in your review settings.