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.

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

Fix #77 codebase remains Java 8 compatible #79

Merged
merged 1 commit into from
Feb 28, 2024
Merged

Fix #77 codebase remains Java 8 compatible #79

merged 1 commit into from
Feb 28, 2024

Conversation

aalmiray
Copy link
Contributor

Fixes #77

@aalmiray
Fix cdevents#77 codebase remains Java 8 compatible
39e4409 
Fixes cdevents#77

Signed-off-by: Andres Almiray <aalmiray@gmail.com>
@aalmiray
Copy link
Contributor Author

aalmiray commented Feb 14, 2024 •
edited
Loading 

$ jarviz bytecode show --file sdk/target/cdevents-sdk-java-0.3.1-SNAPSHOT.jar 
subject: cdevents-sdk-java-0.3.1-SNAPSHOT.jar
Unversioned classes. Bytecode version: 52 (Java 8) total: 341
Versioned classes 9. Bytecode version: 53 (Java 9) total: 1

The single Java9 class is module-info.class as expected.

afrittoli
afrittoli approved these changes Feb 28, 2024
View reviewed changes
Copy link
Contributor

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

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

I don't really fully understand the changes here - I put some questions, but they are probably silly, feel free to ignore me 🙏
For what I can understand it looks good to me.

pom.xml
@@ -34,7 +35,7 @@
<repository.url>git@github.com:${project.github.repository}.git</repository.url>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
<project.build.outputTimestamp>2024-01-11T15:26:09Z</project.build.outputTimestamp>
<project.build.outputTimestamp>${git.commit.author.time}</project.build.outputTimestamp>
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sadly this property is overridden with every release and must be reverted to the expression after a release is posted.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see - perhaps we could include that revert into the release pipeline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. It could be done with a regex. Let me have a look.

@afrittoli
Copy link
Contributor

@rjalander would you like to review this?

@rjalander
Copy link
Contributor

One quick question, Considering Spinnaker is with Java 11, (https://cdeliveryfdn.slack.com/archives/C030SKZ0F4K/p1707936366892439?thread_ts=1707859639.494349&cid=C030SKZ0F4K),

Do we need this PR still to make Java 8 compatible?

@aalmiray
Copy link
Contributor Author

Well, as much as I'd love for everyone to move to recent versions of Java, not everyone has the luxury to do so. With Java 8 having commercial support til 2030 there are still people holding up migration.

@rjalander
Copy link
Contributor

Ok, so good to have a minimum supported version.

I think we need an update here https://github.com/cdevents/sdk-java/blob/main/DEVELOPMENT.md#install-tools for Java version.
If needed, can we have this change in the same PR?

@aalmiray
Copy link
Contributor Author

There is no need for updating the Java version. Java 9 is still the minimum required because ModiTect needs it.

The nice thing about the setup is that the produced bytecode is Java 8 compatible even though the build requires Java 9+

@rjalander rjalander merged commit 3e30a74 into cdevents:main Feb 28, 2024
3 checks passed
@cdevents-bot cdevents-bot added the released Issue has been released label Mar 12, 2024
@cdevents-bot
Copy link
Collaborator

🎉 This issue has been resolved in v0.3.1 (Release Notes)

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

@afrittoli afrittoli afrittoli approved these changes

@rjalander rjalander rjalander approved these changes

Assignees
No one assigned
Labels
released Issue has been released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Codebase must remain Java 8 compatible
4 participants
@aalmiray @afrittoli @rjalander @cdevents-bot