-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
8312882: Update the CONTRIBUTING.md with pointers to lifecycle of a PR #14716
Conversation
👋 Welcome back jglick! A progress list of the required criteria for merging this PR into |
@jglick To determine the appropriate audience for reviewing this pull request, one or more labels corresponding to different subsystems will normally be applied automatically. However, no automatic labelling rule matches the changes in this pull request. In order to have an "RFR" email sent to the correct mailing list, you will need to add one or more applicable labels manually using the /label pull request command. Applicable Labels
|
Hello Jesse, I've opened https://bugs.openjdk.org/browse/JDK-8312882 to track this change. Could you update the title of this PR to |
CONTRIBUTING.md
@jglick Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information. |
By the way the bots seem to contradict themselves:
and a previous failing
implies that there would be no way to correct a mistaken commit message! Perhaps the bot messages should be adjusted to refer to the PR title, which I think is what they are actual checking, rather than “the commit message” (which is especially ambiguous if there are multiple commits). |
/label add build |
@jaikiran |
Webrevs
|
I suspect in the absence of the JBS issue in the PR title the bot/jcheck was looking for one in the commit message. Unfortunately we can't see the failed check now. |
I can see how this looks confusing. Here is what's actually happening. The Regarding this change, I think |
In that case shall I close this PR, and someone with permission to do so could reword JDK-8312882 to that effect? |
The official, and only up-to-date resource for how to contribute changes to the OpenJDK is the OpenJDK Developers' Guide.. It's one of the top links on the OpenJDK home page so it shouldn't be that difficult to find. One problem is of course that the CONTRIBUTING.md which is the link just before the guide refers to the slightly outdated building.md ... That's the problem that should be fixed here. The text about how to contribute in building.md should be removed as it doesn't belong in that document at all, and is wrong. |
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'm changing my mind and agreeing with Jesper.
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.
Looks good to me. I would prefer if another reviewer approves this change as well even though the bots might accept that only one reviewer approved.
|
@jglick This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 1 new commit pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@erikj79, @JesperIRL) but any other Committer may sponsor as well. ➡️ To flag this PR as ready for integration with the above commit message, type |
/integrate |
/sponsor |
Going to push as commit 42758cb.
Your commit was automatically rebased without conflicts. |
@JesperIRL @jglick Pushed as commit 42758cb. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
@jaikiran in #12871 (comment) pointed me to a resource I had not previously found. https://openjdk.org/contribute is fine at a high level but still says things like
which clearly predates Skara, and does not apparently link to anything more current. Whereas this https://github.com/openjdk/jdk/blob/master/doc/building.md is thorough and discoverable, finding accurate guidelines for proposing pull requests was tough.
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/14716/head:pull/14716
$ git checkout pull/14716
Update a local copy of the PR:
$ git checkout pull/14716
$ git pull https://git.openjdk.org/jdk.git pull/14716/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 14716
View PR using the GUI difftool:
$ git pr show -t 14716
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/14716.diff
Webrev
Link to Webrev Comment