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

Fix bytecode compatible back to 52(JDK8) #473

Merged
merged 1 commit into from
Dec 21, 2023

Conversation

shifujun
Copy link
Contributor

@shifujun shifujun commented Dec 18, 2023

This project need JDK 11 to compile, but target to JDK 8.

IDEA need uncheck "use --release" in Preferences |
Build, Execution, Deployment | Compiler | Java Compiler
, to build success.

fix #470

Copy link
Contributor

@wuwen5 wuwen5 left a comment

Choose a reason for hiding this comment

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

.idea/compiler.xml This file should not be checked in, it is in .gitignore

@shifujun
Copy link
Contributor Author

.idea/compiler.xml This should not be submitted, it is in .gitignore

IDEA need it. I can't find another way to make IDEA auto import work.

In addition, being in .gitignore does not prevent it from working properly. Users will usually no longer trigger modifications to it.

@wuwen5
Copy link
Contributor

wuwen5 commented Dec 19, 2023

This is just a configuration file for a personal local development tool that is independent of the source code and build,It's generally recommended not to include IDE-specific configuration files in the repository, as they are specific to each developer's local environment.

@shifujun
Copy link
Contributor Author

  1. Adding no IDE configuration to git is also my goal.
  2. Popular IDE(IDEA)can import this project is very helpful for getting more contributors.
  3. Compile JDK 9 source to JDK 8 target is not standard, so I can't blame IDEA cannot auto import.
  4. Not all IDE config file is personal relative. .idea/compiler.xml is generic.

@hazendaz
Copy link

@shifujun How about you bridge the divide here. No one wants to see anything .idea checked in. So why not add to the README for IDEA users they need to do that and put the same in change log once release is tagged and updated? That seems like a compromise that will fix this back for now.

For reference, I'm over at https://github.com/mybatis/ and we shade this and cannot upgrade to this version without raising to java 11. We are still discussing where we want to raise but this report https://newrelic.com/resources/report/2023-state-of-the-java-ecosystem shows there is still a lot of life left with java 8. So I think don't press people to have idea configuration files on a repo. That isn't helpful, but if you need them to know how, document it instead.

Verified

This commit was signed with the committer’s verified signature.
myii Imran Iqbal
This project need JDK 11 to compile, but target to JDK 8.

IDEA need uncheck "use --release" in Preferences |
Build, Execution, Deployment | Compiler | Java Compiler
, to build success.

fix jboss-javassist#470
@shifujun shifujun force-pushed the fix_bytecode_version branch from d7912e0 to dc5aefe Compare December 20, 2023 00:36
.idea/compiler.xml Outdated Show resolved Hide resolved
@LowAmmo
Copy link

LowAmmo commented Dec 20, 2023

@wuwen5 - We need to get this fix to get some of our applications working again. Any chance this can be merged and released ASAP?

@wuwen5
Copy link
Contributor

wuwen5 commented Dec 21, 2023

@LowAmmo I'm not a project member, but I share your sense of urgency. I hope the fix can be merged and released as soon as possible.

@chibash Could you help us release the version?

@chibash chibash merged commit fe634f3 into jboss-javassist:master Dec 21, 2023
2 checks passed
@chibash
Copy link
Member

chibash commented Dec 21, 2023

I'll release this by this weekend. Is this too late?

@LowAmmo
Copy link

LowAmmo commented Dec 21, 2023

@chibash - I appreciate the quick response on the merge! Thank you!

At least for me, the sooner the better, but I totally understand if you need to wait a few days.

-Thanks!

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.

7 participants