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

Jakarta version #186

Closed
mann-ed opened this issue May 25, 2024 · 9 comments · Fixed by #194
Closed

Jakarta version #186

mann-ed opened this issue May 25, 2024 · 9 comments · Fixed by #194

Comments

@mann-ed
Copy link
Contributor

mann-ed commented May 25, 2024

I was updating my projects to Jakarta and this is one of the projects that does not have a Jakarta version. I spent some time and created a fork and few sub projects to have jakarta support.

Pull Request incoming, so we can discuss about changes.

@mann-ed
Copy link
Contributor Author

mann-ed commented May 25, 2024

For javax i had to use java 11 to build
mvn clean test integration-test -pl :javax-property-inject
Then for jakarta i'm using java 21
mvn clean package install integration-test test -pl :jakarta-property-inject,jakarta-property-inject-test

pull Request #187

This should help anyone trying to build my fork.

@mann-ed
Copy link
Contributor Author

mann-ed commented May 25, 2024

I was not happy with the initial change to get Jakarta support. So i worked on it a little more after some sleep and have a version that is just the single project. I have added profiles to build for javax and jakarta. Going to create new pull request for that

@MikeEdgar
Copy link
Member

@mann-ed , I'm away from my computer for a few days and I intend to review/respond to your work some time next week. Thank you

@mann-ed
Copy link
Contributor Author

mann-ed commented Jun 5, 2024

@MikeEdgar I see the build is failing. It's not giving me much in logs to go by to help resolve the issue, or i'm not looking in the right place. I know the jakarta command can't run on java 8. Let me know what i can do to move this on. I did see i left my personal artifactory repo in the pom.xml file. Sorry about that. I will get it pulled out now.

@MikeEdgar
Copy link
Member

@mann-ed , I wonder if now is the time to just rip off the band-aid and move to Jakarta only. It would make things a bit simpler and avoid the need to build for both javax and jakarta packages. What do you think?

@mann-ed
Copy link
Contributor Author

mann-ed commented Jun 7, 2024

@MikeEdgar That's a tough question. I think the build for this one is pretty straight forward, not any code changes that developers need to make. If we ever introduced changes that are only Jakarta only, then i think javax should be drop.

I know how some shops don't update fast, so they are stuck with javax. Then again this could be another reason for them to update to the new java release.
Then again, as a developer we would need to keep old javax java around to develop with. So it might be that with this new major release javax is now dropped. Would make development easier for us.

I know not much help am i? LOL

@MikeEdgar
Copy link
Member

@mann-ed I'm thinking it might be time to just make the change. The Jakarta namespace has been out for several years now, so I think it's fair to say that when there are new features to property-inject, developers need to update to a more recent version of the platform.

A good option would probably be to continue generating Java 8 compatible bytecode (which should work with Jakarta EE 9), but only run the CI on 17/21 in this repository, since several of the dependencies have moved to 17 in their current versions.

@mann-ed
Copy link
Contributor Author

mann-ed commented Jun 8, 2024

@MikeEdgar Do you want me to create a new branch, update the code and pom file, then create a new pull request?
I'm out teaching this weekend, but i can get to it on Monday.

@MikeEdgar
Copy link
Member

I think a separate PR would be good. Thank you for looking into this, and of course there is no rush at all.

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 a pull request may close this issue.

2 participants