Skip to content

Upgrade google formatter to v1.26.0 - Requires Java 17 #223

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Upgrade google formatter to v1.26.0 - Requires Java 17 #223

wants to merge 2 commits into from

Conversation

mkurz
Copy link
Member

@mkurz mkurz commented Apr 10, 2025

No description provided.

@ihostage
Copy link

Matthias, do you want to merge this before next release?

@mkurz
Copy link
Member Author

mkurz commented Apr 10, 2025

I want to merge this an publish is as v1.0.0

@mkurz
Copy link
Member Author

mkurz commented Apr 10, 2025

So 0.9 is for Java 8, 0.10 is for Java 11 and 1.x is for Java 17+

@mkurz
Copy link
Member Author

mkurz commented Apr 10, 2025

Nooo.... --add-exports is needed now...

For the scripted tests we can add them do the scriptedLaunchOpts I guess. Will do that later, need to head out for an appointment

ihostage
ihostage reviewed Apr 10, 2025
View reviewed changes
.github/workflows/ci.yml
@@ -12,11 +12,11 @@ jobs:
matrix:
include:
- os: ubuntu-latest
java: 11
java: 17
distribution: zulu
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using temurin or liberica? 😄

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just it's strange to use different distributions for build and release workflows 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, not sure why zulu was used by Eugene. I remember there was a problem with temurin 8 not being available for the newer ubuntu image or something, or was it for the arm image. Whatever, we can use temurin.

@ihostage
Copy link

@mkurz Please, don't release 1.0.0 yet 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ihostage ihostage ihostage left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mkurz @ihostage