Remove support for jextract -interactive - #18230
Conversation
keithc-ca
left a comment
There was a problem hiding this comment.
Please remove the (now) unused code:
- Main.runInteractive()
- supporting imports
- Main.doCommand() and associated native code, exports, etc. (jextraceglue.c, jextractnatives_internal.h, jextractnatives.c, jextractnatives_api.h, module.xml, CMakeLists.txt)
See https://github.com/eclipse-openj9/openj9/blob/master/doc/build-instructions/Build_Instructions_V21.md for how to set up your system and build (Java 21 in this case) locally. |
|
@keithc-ca Thanks for pointing these errors out. I am working on the exports, and have made the changes you mentioned in the Java source code. |
|
@keithc-ca Thanks for the reviews. I hope everything is in order now. |
|
The overall change looks good. I think it's time to rebase and squash to a single commit. |
|
@keithc-ca Hi Keith. I have squashed all the commits into one. Thanks a lot for your cooperation and patience, and apologies for any trouble I may have caused. I learnt a lot of new things while I contributed to this issue, and I will put my knowledge to good use. |
|
Thank you for squashing to a single commit. It would be nice if you could rebase on a more recent commit on the master branch, but that's less important. I would like you to improve the commit message; see [1] for guidance. The first line should summarize what merging the commit will do (the summary here, "Remove support for jextract -interactive", would be fine). Normally, the body would provide more detail, but I'm not sure there's much more to say. On the other hand, I don't think it's helpful to have text like "removed blank line" or "removed another extra line", etc. [1] https://github.com/eclipse-openj9/openj9/blob/master/CONTRIBUTING.md |
4317b15 to
bb5ffa4
Compare
|
First, a minor nit: the first line of the commit message should start with "Remove", not "Removed". I'm not sure where the other 19 commits came from, but I think what you want to do is git fetch --all
git rebase -i 5570f22bf9daThis will open an editor showing You want to change "pick" to "drop" on the first line. Save the file and exit the editor. Git will skip commit 29d12dea56a and create a new commit with the other changes. You'll want to remove what I expect is some intermediate temporary file git rm runtime/compiler/build/scripts/RCa05068and then amend that commit Again, git will open an editor allowing you to update the (amended) commit message; here's your change to remove that unwanted "d". |
This commit will remove support for '-interactive' flag from jextract executable. 1. Removed the method responsible for dealing with '-interactive' flag 2. Removed unused imports required by said method 3. Removed unused exports of said method in other files 4. Removed the following files which are now unused: runtime/jextractnatives/jextractnatives_internal.h runtime/jextractnatives/jextractglue.c 5. Reordered imports and removed empty lines
|
@keithc-ca Apologies for those extra commits being added. I pushed to origin right after fetching master by mistake, and that might be what caused those extra commits to show. Thanks for the help. |
|
Jenkins compile aix,alinux,win jdk17 |
|
Jenkins test sanity amac jdk11 |
Referencing issue #10764
I am not sure how to test this change on my local environment, but I am sure that its support has been ended. Please do point out any error or some additional change that needs to be made and I might have overlooked, and feel free to leave reviews about this contribution. PS: I am new to open source contribution and this will be my first actual pull request, so please provide guidance if I have made any errors.