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.

Sign up for GitHub

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

Jump to bottom

doc: create list for commonly edited files in PRs #19776

Closed
wants to merge 2 commits into from
Closed

doc: create list for commonly edited files in PRs #19776

wants to merge 2 commits into from

Conversation

trivikr
Copy link
Member

@trivikr trivikr commented Apr 3, 2018

This code change arranges commonly edited files in PRs as a list in pull-requests.md
The updated document can be viewed in private branch here

Checklist

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Apr 3, 2018
Trott
Trott approved these changes Apr 3, 2018
View reviewed changes
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

Change LGTM, although honestly, I'd consider deleting the entire paragraph. The document is plenty long and I'm not sure what value that information provides to someone who is in the process of opening a pull request.

@vsemozhetbyt vsemozhetbyt added the meta Issues and PRs related to the general management of the project. label Apr 3, 2018
@trivikr
Copy link
Member Author

trivikr commented Apr 3, 2018 •
edited
Loading

@Trott This could be useful for individuals who haven't decided to create a Pull Request yet and just browsing Contributing documentation. I was one of them before I started contributing to Node.js core
However, I'm not sure what % of users actually do that.

@trivikr
Copy link
Member Author

trivikr commented Apr 3, 2018

CI-lite: https://ci.nodejs.org/job/node-test-pull-request-lite/410/

@trivikr
Copy link
Member Author

trivikr commented Apr 3, 2018

Last CI got aborted
New CI-lite: https://ci.nodejs.org/job/node-test-pull-request-lite/412/

addaleax
addaleax approved these changes Apr 3, 2018
View reviewed changes
doc/guides/contributing/pull-requests.md Outdated
repository includes changes to one or more of the following:
- the C/C++ code contained in the `src` directory
- the JavaScript code contained in the `lib` directory
- the documentation in `docs/api`
Copy link
Member

Choose a reason for hiding this comment

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

(Pre-existing) Typo: docs → doc :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done :)

@trivikr trivikr added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 4, 2018
@trivikr
Copy link
Member Author

trivikr commented Apr 4, 2018

New CI: https://ci.nodejs.org/job/node-test-pull-request-lite/424/

@trivikr
Copy link
Member Author

trivikr commented Apr 6, 2018

Landed in f02e4b9

@trivikr trivikr closed this Apr 6, 2018
@trivikr trivikr deleted the trivikr-patch-1 branch April 6, 2018 01:39
trivikr added a commit that referenced this pull request Apr 6, 2018
@trivikr
doc: create list for commonly edited files in PRs
f02e4b9 
PR-URL: #19776
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this pull request Apr 6, 2018
@trivikr @targos
doc: create list for commonly edited files in PRs
229747e 
PR-URL: #19776
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpinca lpinca lpinca approved these changes

@addaleax addaleax addaleax approved these changes

@Trott Trott Trott approved these changes

@jasnell jasnell jasnell approved these changes

@danbev danbev danbev approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@trivikr @danbev @jasnell @Trott @addaleax @lpinca @vsemozhetbyt @nodejs-github-bot