-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
doc: update COLLABORATOR_GUIDE #2638
doc: update COLLABORATOR_GUIDE #2638
Conversation
(https://github.com/nodejs/node/wiki/Merging-pull-requests-with-Jenkins). | ||
|
||
#### Landing Pull Requests manually (for emergency only) | ||
Pull requests should be landed with Jenkins as documented [here] |
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.
Not sure that we need this sentence repeated again.
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.
How about we restore the links and make the sections with two ## instead of four, but preserve this duplicate disclaimer? Basically I'd like to make sure that if people end-up jumping to this section, that they know that this procedure is deprecated outside of exceptional circumstances.
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.
What if we changed the language just a little bit so it doesn't look like a copy paste job went 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.
Yeah, ok. Will take a stab at it. Suggestions welcome :)
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.
Does this work?
All pull requests should be landed via Jenkins, as documented here. However, sometimes emergency situations arise, and manual intervention is required. Below is a description of a manual procedure for doing the same, should a manual procedure be necessary. Please don't do this unless there is an emergency situation for which the collaborators have agreed that this is necessary.
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.
Love it, thanks! Will make the change...
Updating COLLABORATOR_GUIDE.md to reflect the new workflow for merging PRs with Jenkins.
86f6b3f
to
8fdd0db
Compare
@cjihrig : updated per your feedback. Thank you! |
LGTM |
can we hold off on merging this until we have broad agreement after this trial that this is actually a good thing to lock in? |
Sure. I was thinking that we can always revert it but holding off on it works too. |
Closing for now per #2598 (comment) |
Updating COLLABORATOR_GUIDE.md to reflect the new workflow
for merging PRs with Jenkins.