-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 generateJniConfig step #32829
Fix generateJniConfig step #32829
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
We need the same change applied to https://github.com/quarkusio/quarkus/blob/main/core/deployment/src/main/java/io/quarkus/deployment/steps/NativeImageReflectConfigStep.java as well.
@scrocquesel could you please include the change in this PR? If not I will prepare a follow up.
This is probably one for @Karm to review. I'm not super familiar with this json generation logic. |
Uh...
Why would the Windows test take our latest 21.3. instead of 22.3? Was it because for a couple of days, our 21.3 was the latest release on GH...? |
Re-running the check. And I'll compare the produced json files... |
This comment has been minimized.
This comment has been minimized.
Looks right now. But the flow doesn't do anything when manually restarted as I did:
|
Done |
Failing Jobs - Building 4cd3d42
Full information is available in the Build summary check run. Failures⚙️ Maven Tests - JDK 11 Windows #- Failing: integration-tests/maven
📦 integration-tests/maven✖
✖
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @scrocquesel !
@Karm the version-related failure was due to 21.3.6.0.Final appearing as quarkus/.github/workflows/ci-actions-incremental.yml Lines 696 to 704 in 585ebb2
I think this is good to merge if you have no objection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I unzipped the jars and compared the json files with/without the change for an AWT app and it seems good. THX 🙏
Follow up #32432
When defining both methods and ctors, ctors are discarded. This will merge both.