Skip to content
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

Update Groovy libraries to 3.0.23 #8095

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

matthiasblaesing
Copy link
Contributor

Update bundled Groovy to 3.0.23.

To make the real change better visible, in the first commit the reference files (occurrences|completion|folds|indexed|semantic|ccresult) were cleaned up and files for old java versions were dropped. The baseline was raised to JDK 17. In the second step the Groovy update was done, the reference files regenerated for JDK 17 and additional reference files for JDK 21 generated.

The real change can be observed in the changed reference files in the second commit. Only the findAll methods gained a more precise return type (from Collection to List).

@matthiasblaesing matthiasblaesing added Groovy ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Dec 27, 2024
@matthiasblaesing matthiasblaesing added this to the NB25 milestone Dec 27, 2024
…ult) files

1. Check that tests run clean with the base JDK (for this commit 17)
2. Remove all existing reference files:
   find . -regextype egrep -regex '.*\.(occurrences|completion|folds|indexed|semantic|ccresult)'  -exec rm {} \;
3. Rebuild the reference files by running unittests
4. Rerun unittests to verify all green
Return type from findAll changed from Collection to List

Workflow:

1. Update the groovy version
2. Remove all reference files:
   find . -regextype egrep -regex '.*\.(occurrences|completion|folds|indexed|semantic|ccresult)'  -exec rm {} \;
3. Rebuild the reference files by running the unittests with the base
   JDK (17 for this commit)
4. Review the difference
5. Commit the change with the updated reference files
6. Add JDK specific variants by:
   6a: Removing all existing reference files:
       find . -regextype egrep -regex '.*\.(occurrences|completion|folds|indexed|semantic|ccresult)'  -exec rm {} \;
   6b: Rerun unittests with the target JDK
   6c: Move the changed reference files to their version specific name
       (21 is the JDK version in this case and needs to be adjusted):
       for i in `git status -s | cut --delimiter " " -f 3`; do mv $i ${i%.*}.21.completion; git checkout $i; done
   6d: Amend the changes to the base commit
@mbien mbien added the Upgrade Library Library (Dependency) Upgrade label Dec 28, 2024
@mbien
Copy link
Member

mbien commented Dec 29, 2024

hmm. I probably should have done this already in #7415 as cleanup, instead of making sure that everything passes between JDK 8 and 17 (even on EOL JDKs).

Maintaining golden files only for JDK 17 and 21 is probably fine for the groovy modules. Some tests will likely fail on JDKs 18-20 but the 17k removed lines are very enticing since it would reduce maintenance overhead.

should we let CI also test on JDK 21 when Groovy is set?

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 1e5be8f..55b0919 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -1827,7 +1827,9 @@
     timeout-minutes: 60
     strategy:
       matrix:
-        java: [ '17' ]
+        java: [ '17', '21' ]
+        exclude:
+          - java: ${{ github.event_name == 'pull_request' && 'nothing' || '21' }}
       fail-fast: false
     steps:

@matthiasblaesing
Copy link
Contributor Author

If noone objects, I intent to merge this next weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) Groovy Upgrade Library Library (Dependency) Upgrade
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants