FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: reorganize COLLABORATOR_GUIDE.md by Trott · Pull Request #15710 · nodejs/node · GitHub

/ node Public

doc: reorganize COLLABORATOR_GUIDE.md - #15710

Closed
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:collab-guid-reorg
ConversationCommits (1)ChecksFiles changed
Closed

doc: reorganize COLLABORATOR_GUIDE.md#15710
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:collab-guid-reorg

Conversation

Trott commented Oct 1, 2017

Copy link
Copy Markdown
Member

Based on feedback during a recent collaborator onboarding, move the
metadata description closer to where the instructions for editing a
commit message are.

Indicate which metadata are required and which are optional.

Make the punctuation more consistent. Previously, sentences prior to
instructions ended in a period, a colon, or no punctuation at all. Colon
seems best, so changed the Technical How-To section to use colons.

Checklist
Affected core subsystem(s)

doc

Trott added the doc Issues and PRs related to the documentations. label Oct 1, 2017

Trott commented Oct 1, 2017

Copy link
Copy Markdown
Member Author

@bmeurer

gibfahn commented Oct 1, 2017

Copy link
Copy Markdown
Member

I agree this info makes sense in the new location, but I think it's also useful to have it in the old, with other info about landing commits.

Maybe just give it it's own heading (so it can be linked against), and then the old location can just contain a link to the new location (or vice versa).

bmeurer approved these changes Oct 1, 2017
View reviewed changes
Comment thread COLLABORATOR_GUIDE.md Outdated

bmeurer Oct 1, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

How about mentioning node-review extension here? It doesn't make sense to even try to extract this info manually.

gibfahn Oct 1, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We already mention node-review in the onboarding doc, though I'm not really sure how to reduce duplication here.

+1 on mentioning node-review here.

Trott Nov 11, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Great idea. I added a sentence and link for node-review. I also updated the contents of the onboarding doc to note that node-review works in Edge browser too. At some point soon, we'll have to add the CLI tool for this that @joyeecheung et al. have been working on.

lpinca reviewed Oct 1, 2017
View reviewed changes
Comment thread COLLABORATOR_GUIDE.md Outdated

lpinca Oct 1, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Nit: multiple Refs: lines may also be added.

Trott Nov 11, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Good point. Changed it to be "one or more".

refack reviewed Oct 1, 2017
View reviewed changes
Comment thread COLLABORATOR_GUIDE.md Outdated

refack Oct 1, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I thought the order in the previous revision was invariant, i.e.:

  1. PR-URL
  2. [Fixes]
  3. [Refs]
  4. Reviewed-By

Anyway that's the order node-review formats the metadata.

Trott Nov 11, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@refack I didn't mean for the list to specify ordering, but since that's something you inferred and I doubt you will be alone in doing that, I've change it back to the ordering you have above.

Trott force-pushed the collab-guid-reorg branch 3 times, most recently from 9e9f472 to 4d7c723 Compare November 11, 2017 21:52

Trott commented Nov 11, 2017

Copy link
Copy Markdown
Member Author

Maybe just give it it's own heading (so it can be linked against), and then the old location can just contain a link to the new location (or vice versa).

@gibfahn Good suggestion. I added an anchor using <a name=... (since valid HTML is valid markdown) and put in a single sentence with a link.

Trott force-pushed the collab-guid-reorg branch 3 times, most recently from 61512a1 to 3b85c56 Compare November 11, 2017 22:43
refack reviewed Nov 11, 2017
View reviewed changes
Comment thread doc/onboarding.md Outdated

refack Nov 11, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Extra! Extra! Read all about it!

refack Nov 11, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

New CLI tool node-core-utils fetches the needed meta data from the command line.

refack approved these changes Nov 11, 2017
View reviewed changes
Comment thread doc/onboarding.md Outdated

refack Nov 11, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

New CLI tool node-core-utils fetches the needed meta data from the command line.

Trott force-pushed the collab-guid-reorg branch from 3b85c56 to 7e0ed66 Compare November 12, 2017 22:15
doc: reorganize COLLABORATOR_GUIDE.md
4a422be 
Based on feedback during a recent collaborator onboarding, move the
metadata description closer to where the instructions for editing a
commit message are.

Indicate which metadata are required and which are optional.

Make the punctuation more consistent. Previously, sentences prior to
instructions ended in a period, a colon, or no punctuation at all. Colon
seems best, so changed the Technical How-To section to use colons.
Trott force-pushed the collab-guid-reorg branch from 7e0ed66 to 4a422be Compare November 12, 2017 22:16

Trott commented Nov 12, 2017

Copy link
Copy Markdown
Member Author

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

Trott added a commit to Trott/io.js that referenced this pull request Nov 13, 2017
doc: reorganize COLLABORATOR_GUIDE.md
5df47d0 
Based on feedback during a recent collaborator onboarding, move the
metadata description closer to where the instructions for editing a
commit message are.

Indicate which metadata are required and which are optional.

Make the punctuation more consistent. Previously, sentences prior to
instructions ended in a period, a colon, or no punctuation at all. Colon
seems best, so changed the Technical How-To section to use colons.

PR-URL: nodejs#15710
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>

Trott commented Nov 13, 2017

Copy link
Copy Markdown
Member Author

Landed in 5df47d0.

Trott closed this Nov 13, 2017
evanlucas pushed a commit that referenced this pull request Nov 13, 2017
doc: reorganize COLLABORATOR_GUIDE.md
54768f5 
Based on feedback during a recent collaborator onboarding, move the
metadata description closer to where the instructions for editing a
commit message are.

Indicate which metadata are required and which are optional.

Make the punctuation more consistent. Previously, sentences prior to
instructions ended in a period, a colon, or no punctuation at all. Colon
seems best, so changed the Technical How-To section to use colons.

PR-URL: #15710
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
evanlucas mentioned this pull request Nov 13, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Nov 17, 2017
doc: reorganize COLLABORATOR_GUIDE.md
3755a0d 
Based on feedback during a recent collaborator onboarding, move the
metadata description closer to where the instructions for editing a
commit message are.

Indicate which metadata are required and which are optional.

Make the punctuation more consistent. Previously, sentences prior to
instructions ended in a period, a colon, or no punctuation at all. Colon
seems best, so changed the Technical How-To section to use colons.

PR-URL: #15710
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 17, 2017
doc: reorganize COLLABORATOR_GUIDE.md
ccd3646 
Based on feedback during a recent collaborator onboarding, move the
metadata description closer to where the instructions for editing a
commit message are.

Indicate which metadata are required and which are optional.

Make the punctuation more consistent. Previously, sentences prior to
instructions ended in a period, a colon, or no punctuation at all. Colon
seems best, so changed the Technical How-To section to use colons.

PR-URL: #15710
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
gibfahn mentioned this pull request Nov 21, 2017
Merged
MylesBorins mentioned this pull request Nov 21, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
doc: reorganize COLLABORATOR_GUIDE.md
30fa3c0 
Based on feedback during a recent collaborator onboarding, move the
metadata description closer to where the instructions for editing a
commit message are.

Indicate which metadata are required and which are optional.

Make the punctuation more consistent. Previously, sentences prior to
instructions ended in a period, a colon, or no punctuation at all. Colon
seems best, so changed the Technical How-To section to use colons.

PR-URL: #15710
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
doc: reorganize COLLABORATOR_GUIDE.md
4583f1b 
Based on feedback during a recent collaborator onboarding, move the
metadata description closer to where the instructions for editing a
commit message are.

Indicate which metadata are required and which are optional.

Make the punctuation more consistent. Previously, sentences prior to
instructions ended in a period, a colon, or no punctuation at all. Colon
seems best, so changed the Technical How-To section to use colons.

PR-URL: #15710
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Trott deleted the collab-guid-reorg branch January 13, 2022 22:47
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
lpinca lpinca approved these changes
+4 more reviewers
gibfahn gibfahn left review comments
refack refack approved these changes
bmeurer bmeurer approved these changes
vsemozhetbyt vsemozhetbyt approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL