-
-
Notifications
You must be signed in to change notification settings - Fork 32.6k
doc: improve onboarding instructions #59159
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
Merged
+31
−7
Merged
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -230,11 +230,13 @@ needs to be pointed out separately during the onboarding. | |
labels. The `fast-track` label should cause the Node.js GitHub bot to post a | ||
comment in the pull request asking collaborators to approve the pull request | ||
by leaving a 👍 reaction on the comment. | ||
* Optional: Run CI on the pull request. Use the `node-test-pull-request` CI | ||
* Optional: Run Jenkins CI on the pull request. Use the [`node-test-pull-request`][] | ||
task. As a convenience, you may apply the `request-ci` label to the pull | ||
request to have a GitHub Actions workflow start the Jenkins CI task for you. | ||
* After two Collaborator approvals for the change and two Collaborator approvals | ||
for fast-tracking, land the PR. | ||
for fast-tracking, land the PR. If you have started a full Jenkins CI, cancel it | ||
from the Jenkins UI since the PR is a doc-only change and does not need | ||
a full CI run, it is just run as an exercise. | ||
* If there are not enough approvals within a reasonable time, consider the | ||
single approval of the onboarding TSC member sufficient, and land the pull | ||
request. | ||
|
@@ -245,6 +247,22 @@ needs to be pointed out separately during the onboarding. | |
* [`core-validate-commit`][] automates the validation of commit messages. | ||
This will be run during `git node land --final` of the [`git-node`][] | ||
command. | ||
* Normally you can just use the `commit-queue` label to have the | ||
commit queued for landing by the Node.js GitHub bot. But as exercise it is | ||
also useful to learn how to land commits manually in case the bot or the CI | ||
is broken. | ||
* If you are landing the commit manually, to make it appear as "Merged" on GitHub, | ||
after you prepare the landed commit on the local `main` branch, run this: | ||
|
||
```bash | ||
git checkout your-pr-branch | ||
git reset --hard main # Synchronize your PR branch with the local main branch you just prepared. | ||
git push --force-with-lease your-fork-remote your-pr-branch # Update the PR branch in your fork. | ||
git checkout main | ||
git push upstream main # Push the landed commit to the upstream main branch. | ||
``` | ||
GitHub will automatically detect that the PR branch is now identical to the | ||
`main` branch and will mark the PR as "Merged". | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to mention There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, shouldn't that be implemented in NCU so that there's no need to install gh separately? |
||
|
||
## Final notes | ||
|
||
|
@@ -253,11 +271,14 @@ needs to be pointed out separately during the onboarding. | |
* Almost any mistake you could make can be fixed or reverted. | ||
* The existing collaborators trust you and are grateful for your help! | ||
* Other repositories: | ||
* <https://github.com/nodejs/TSC> | ||
* <https://github.com/nodejs/build> | ||
* <https://github.com/nodejs/nodejs.org> | ||
* <https://github.com/nodejs/Release> | ||
* <https://github.com/nodejs/citgm> | ||
* <https://github.com/nodejs/TSC>: Governance discussions and TSC votes | ||
* <https://github.com/nodejs/build>: Build infrastructure discussions and CI issues | ||
* <https://github.com/nodejs/nodejs.org>: The Node.js website and blog | ||
* <https://github.com/nodejs/Release>: Release management and release planning | ||
* <https://github.com/nodejs/citgm>: Tool for testing popular packages against Node.js changes | ||
* <https://github.com/nodejs/admin>: Administrative issues and requests to changes in the Node.js | ||
GitHub organization (e.g. creating new repositories, new teams, adding organization-wide tokens). | ||
* <https://github.com/nodejs/moderation>: Requests to moderate comments or block spammers. | ||
* The OpenJS Foundation hosts regular summits for active contributors to the | ||
Node.js project, where we have face-to-face discussions about our work on the | ||
project. The Foundation has travel funds to cover [participants' expenses][] | ||
|
@@ -266,6 +287,7 @@ needs to be pointed out separately during the onboarding. | |
repository for details. | ||
* If you are interested in helping to fix coverity reports consider requesting | ||
access to the projects coverity project as outlined in [static-analysis][]. | ||
* If you are interested in helping out with CI flakes, check out the [reliability respository][]. | ||
joyeecheung marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
[Code of Conduct]: https://github.com/nodejs/admin/blob/HEAD/CODE_OF_CONDUCT.md | ||
[Labels]: doc/contributing/collaborator-guide.md#labels | ||
|
@@ -275,7 +297,9 @@ needs to be pointed out separately during the onboarding. | |
[`author-ready`]: doc/contributing/collaborator-guide.md#author-ready-pull-requests | ||
[`core-validate-commit`]: https://github.com/nodejs/core-validate-commit | ||
[`git-node`]: https://github.com/nodejs/node-core-utils/blob/HEAD/docs/git-node.md | ||
[`node-test-pull-request`]: https://ci.nodejs.org/job/node-test-pull-request/ | ||
[participants' expenses]: https://github.com/openjs-foundation/cross-project-council/blob/main/community-fund/COMMUNITY_FUND_POLICY.md#community-fund-rules | ||
[reliability respository]: https://github.com/nodejs/reliability | ||
[set up the credentials]: https://github.com/nodejs/node-core-utils#setting-up-github-credentials | ||
[static-analysis]: doc/contributing/static-analysis.md | ||
[two-factor authentication]: https://help.github.com/articles/securing-your-account-with-two-factor-authentication-2fa/ | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.