Skip to content

Conversation

@juherr
Copy link
Contributor

@juherr juherr commented Apr 3, 2024

Summary:

  • Added CI tests for all Java LTS versions (starting from 8).
  • Integrated examples into the CI workflow to improve test coverage. Rework ci pipeline #1735
  • Fixed various issues to ensure all tests pass successfully.

@juherr
Copy link
Contributor Author

juherr commented Aug 25, 2024

@joelittlejohn The build is now working on all Java LTS versions, except for some details:

@juherr
Copy link
Contributor Author

juherr commented Sep 21, 2024

Ping @joelittlejohn

@ctrimble
Copy link
Collaborator

@juherr I would like to try and get this PR moving again.

I have the robolectric issues resolved up to Java 17 in #1704. I am going to pull that into master soon.

Once I have #1704 on master, would you want to pick this back up, rebase to get it current, and try to get something onto master?

@juherr
Copy link
Contributor Author

juherr commented Aug 25, 2025

@ctrimble Sure. Let me know when the main branch is ready.

@ctrimble
Copy link
Collaborator

@juherr the master branch now has JDK 11 and JDK 17 support for the integration tests.

I am still not sure how @joelittlejohn is doing releases for this project. Hopefully he can take a peek and verify that we are not going to break release procedures with this update.

@juherr juherr force-pushed the feature/test-all-lts branch from 705cd51 to a7b8bd4 Compare August 30, 2025 17:10
@juherr juherr force-pushed the feature/test-all-lts branch 5 times, most recently from 21062ba to 1d2b8da Compare August 31, 2025 01:37
@juherr juherr requested a review from ctrimble August 31, 2025 01:43
@ctrimble
Copy link
Collaborator

I have added #1722 to get the maven wrapper into the project. This commit cherry-picked over without any changes needed.

@ctrimble
Copy link
Collaborator

#1722 is now on master. @juherr is there anything else in here that is not specifically about adding JDK 11,17,21 support?

@ctrimble
Copy link
Collaborator

ctrimble commented Oct 17, 2025

@juherr I have push a rebased branch to my fork. Please force push https://github.com/ctrimble/jsonschema2pojo/tree/feature_test-all-lts onto your branch when you get a chance.

It looks like we could also move the following changes to other PRs as improvements:

  • Upgrading Gradle/gradlew in the gradle module.
  • Making examples more consistent.
    • Moving the maven example to be more consistent with other examples.
    • The dependabot updates for examples
    • Naming cleanup in the maven example

This would leave us with just the changes that are about this feature. That should make this PR much easier to review and get committed. I am going to see if I can get those broken out and onto master.

@ctrimble
Copy link
Collaborator

#1723 was added with the gradle update. The rebased branch in my repo was also updated.

I am going to gather up these example updates into a PR next.

@juherr
Copy link
Contributor Author

juherr commented Oct 17, 2025

Please check #1714 first. It should help to review too.

@juherr
Copy link
Contributor Author

juherr commented Oct 17, 2025

@juherr I have push a rebased branch to my fork. Please force push https://github.com/ctrimble/jsonschema2pojo/tree/feature_test-all-lts onto your branch when you get a chance.

Ok, I will do that.

@ctrimble
Copy link
Collaborator

Please check #1714 first. It should help to review too.

Thanks for the heads up on that PR. I thought we needed to merge that after this PR. I now understand.

I am hoping to get all of this merged in smaller chunks. It is good work, but it is too much to do in one shot.

@juherr juherr force-pushed the feature/test-all-lts branch from 3995a8c to c75de38 Compare October 17, 2025 11:14
@juherr
Copy link
Contributor Author

juherr commented Oct 17, 2025

@ctrimble Done. JDK 25 should be added now.
By the way, I can’t remember why the Maven example is still limited to JDK 8. Maybe worth trying again?

@ctrimble ctrimble mentioned this pull request Oct 17, 2025
@ctrimble
Copy link
Collaborator

@juherr the example updates are now on master. I have updated https://github.com/ctrimble/jsonschema2pojo/tree/feature_test-all-lts again. Please force push those changes to your branch and we should be ready to review this change in isolation.

@juherr juherr force-pushed the feature/test-all-lts branch 7 times, most recently from 032dbf1 to 1b988c4 Compare October 17, 2025 20:28
@ctrimble
Copy link
Collaborator

ctrimble commented Oct 17, 2025

@juherr Dude, the enthusiasm is great, but please stop adding things. Take a minute to update the description box at the top of this PR, clearly define what this PR now accomplishes and stick to just that.

Note: It is fine if we want to include JDK 25 now, since we really have this narrowed down to just version support, but the change in scope is killing me.

@juherr juherr force-pushed the feature/test-all-lts branch from 1b988c4 to ff25965 Compare October 17, 2025 20:43
@juherr
Copy link
Contributor Author

juherr commented Oct 18, 2025

@ctrimble The goal of this pr has always been the same: to ensure the project runs correctly on all LTS versions.
Unfortunately, a new LTS was released just a month ago 😅

@juherr
Copy link
Contributor Author

juherr commented Oct 18, 2025

@ctrimble As you did over the last few days (thanks again for that!), an additional PR could introduce all the examples as a CI step.
The final step would then be to run tests across all LTS versions.

@ctrimble
Copy link
Collaborator

@juherr

Take a minute to update the description box at the top of this PR, clearly define what this PR now accomplishes and stick to just that.

@juherr
Copy link
Contributor Author

juherr commented Oct 18, 2025

@juherr

Take a minute to update the description box at the top of this PR, clearly define what this PR now accomplishes and stick to just that.

Done

@juherr juherr mentioned this pull request Oct 18, 2025
@juherr
Copy link
Contributor Author

juherr commented Oct 18, 2025

@ctrimble As you did over the last few days (thanks again for that!), an additional PR could introduce all the examples as a CI step. The final step would then be to run tests across all LTS versions.

Done here: #1735

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants