Navigation Menu

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

Enforcing JDK 11 on release #148

Merged
merged 1 commit into from Apr 17, 2021

Conversation

mkarg
Copy link
Collaborator

@mkarg mkarg commented Apr 5, 2021

Enforcing the use of JDK 11 on release guarantees that the published Multi-Release JAR contains ALL code versions: JDK 8 (default fallback), JDK 9 (enhanced NIO) and JDK 10 (even further enhanced NIO). Without this enforcement, there would be a risk of not having some of these versions packed into the MRJAR due to the way the POM checks for the existence of supported JDK levels using JDK-enabled profiles.

This PR was requested by @olamy in #120 (comment), and is effective only in the plexus-release profile as requested by @rfscholte in #148 (comment). While it technically would be enough to just enforce JDK 10, and while it would be more future-proof to even enforce JDK 16, it follows @olamy 's preference mentioned in #120 (comment), hence enforce JDK 11 instead.

pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
@mkarg mkarg marked this pull request as draft April 5, 2021 16:36
@mkarg
Copy link
Collaborator Author

mkarg commented Apr 5, 2021

@olamy @michael-o Please review this PR.

I will keep it in draft state until you vote with +1 and I pretend to squash it before merging. :-)

pom.xml Outdated Show resolved Hide resolved
@olamy
Copy link
Member

olamy commented Apr 5, 2021

my understanding of draft vs not draft is draft means not ready for review vs ready for review.
I don't really understand why you are using such workflow

@mkarg
Copy link
Collaborator Author

mkarg commented Apr 6, 2021

My understanding of draft is "not ready for merge, but ready for review", so I did that to express that the code possibly will change due to the incoming change requests.

@olamy
Copy link
Member

olamy commented Apr 7, 2021

My understanding of draft is "not ready for merge, but ready for review", so I did that to express that the code possibly will change due to the incoming change requests.

interesting point of view and workflow.
So what is non draft status? "ready for merge but too late for review" so nobody can make review anymore?

@mkarg
Copy link
Collaborator Author

mkarg commented Apr 7, 2021

non-draft means "ready to merge". Nothing else. Can still be reviewed. The sole point about "draft" mode is that it prevents clicking "merge" too early.

@mkarg
Copy link
Collaborator Author

mkarg commented Apr 12, 2021

@olamy @rfscholte This PR introduces the enforcement you requested. I answered your questions, and you did not further respond. So I would like to propose that you post a +1 when you want me to merge this one, or clearly request changes otherwise.

@slachiewicz
Copy link
Member

@olamy
Copy link
Member

olamy commented Apr 13, 2021

please follow the usual workflow and remove draft status saying it's ready for review.
otherwise there is no need of a draft status...
We use that here and probably it's used in 99% of the github project.

pom.xml Outdated Show resolved Hide resolved
@mkarg
Copy link
Collaborator Author

mkarg commented Apr 17, 2021

please follow the usual workflow and remove draft status saying it's ready for review.
otherwise there is no need of a draft status...
We use that here and probably it's used in 99% of the github project.

In other projects I am contributing to, I was explicitly asked to keep in draft mode unless no more commits are expected to be put ontop (i. e. the committers are coaching the PR author towards a state that looks like all would be +1, hence the PR changes heavily). So please clearly tell me how that "usual" way is like. Currently I am still adding commits ontop as while the reviews run, change request do appear. Hence, when do committers know when to finally vote if not by the day it turns from draft to non-draft? So shall this really happen outside of draft mode? Just need to get a confirmation on that, and I will do that. Again, this is not that I want to propose another process or want to discuss yours, I just need to know how you want it. :-)

@mkarg
Copy link
Collaborator Author

mkarg commented Apr 17, 2021

Fixed last change request, no more discussions unanswered, no -1 so far => switching from draft to non-draft now. Thanks to everybody for the feedback to the draft. Committers, please cast your VOTE now! :-)

@mkarg mkarg marked this pull request as ready for review April 17, 2021 13:06
Enforcing the use of JDK 11 on release guarantees that the published Multi-Release JAR contains ALL code versions: JDK 8 (default fallback), JDK 9 (enhanced NIO) and JDK 10 (even further enhanced NIO). Without this enforcement, there would be a risk of not having some of these versions packed into the MRJAR due to the way the POM checks for the existence of supported JDK levels using JDK-enabled profiles.
@slachiewicz slachiewicz merged commit 2db9aac into codehaus-plexus:master Apr 17, 2021
@mkarg mkarg deleted the enforce-jdk11-on-release branch April 17, 2021 13:36
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.

None yet

6 participants