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

[HOLD] Fix for WFCORE-5835, Missing modular export when enabling dns.ping protocol #5005

Closed
wants to merge 1 commit into from

Conversation

jfdenise
Copy link
Contributor

@jfdenise jfdenise commented Mar 14, 2022

Issue: https://issues.redhat.com/browse/WFCORE-5835
This PR is on hold, do not merge. Fix could be applied differently.

@jfdenise
Copy link
Contributor Author

@ropalka , if you could give an eye. Once merged it would conflict with the PR you opened: #4966

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Mar 14, 2022
@jfdenise jfdenise changed the title Fix for WFCORE-5835, Missing modular export when enabling dns.ping protocol [HOLD] Fix for WFCORE-5835, Missing modular export when enabling dns.ping protocol Mar 15, 2022
@yersan yersan added the hold Do not merge this PR label Mar 15, 2022
Copy link
Contributor

@ropalka ropalka left a comment

Choose a reason for hiding this comment

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

LGTM @jfdenise , just FYI my PR you mentioned above is not going to be applied soon. It turned into discussion PR for now.

@jfdenise
Copy link
Contributor Author

@ropalka , thank-you. I put this PR on hold for now, there are some discussions for this one too.

@jfdenise
Copy link
Contributor Author

@bstansberry @pferraro should we still hold this PR? Thank-you.

@bstansberry
Copy link
Contributor

@jfdenise -- @pferraro can confirm but IIRC this what not a change we wanted to make outside of the cloud context.

@jfdenise
Copy link
Contributor Author

@bstansberry , Ok, that is something that we can handle in the cloud if @pferraro confirms.

@bstansberry
Copy link
Contributor

BTW, my concern about this was the reference to the jdk.* module and the impact that might have on users who want to run with a slimmed JVM. Unfortunately you can't add JPMS settings that refer to non-existent modules, so it's not safe to just add stuff.

@jfdenise
Copy link
Contributor Author

jfdenise commented May 3, 2022

Closing, will be handled at the cloud level.

@jfdenise jfdenise closed this May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-ok Dependencies have been checked, and there are no significant changes hold Do not merge this PR
Projects
None yet
4 participants