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: document use of Refs: for references #10670

Closed
wants to merge 1 commit into from
Closed

doc: document use of Refs: for references #10670

wants to merge 1 commit into from

Conversation

gibfahn
Copy link
Member

@gibfahn gibfahn commented Jan 6, 2017 •
edited
Loading

Standardise on Refs:

Vote on this straw poll

http://www.strawpoll.me/12049231

(see #10670 (comment))

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

It seems that using Refs: or Ref: in commit messages isn't documented anywhere. I'd suggest we standardise on one or the other. I picked Refs: as it's more common, but either is fine.

➜  node git:(master) git log | grep Ref: | wc -l
     189
➜  node git:(master) git log | grep Refs: | wc -l
     262

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. lts-watch-v6.x labels Jan 6, 2017
@gibfahn gibfahn mentioned this pull request Jan 6, 2017
Closed
2 tasks
@mscdex mscdex added the meta Issues and PRs related to the general management of the project. label Jan 6, 2017
@MylesBorins
Copy link
Contributor

nit: Refs makes more sense if it is a single title for multiple references. We usually include it in each line so Ref makes more sense to me

BIAB gotta paint a shed

@gibfahn
Copy link
Member Author

gibfahn commented Jan 6, 2017

I see it as a verb if it has an s, as in this commit references/refs this link, but am still 100% happy either way. I'd be interested to see what people prefer.

[Friday night bikeshed 🚲 🏠]

@evanlucas
Copy link
Contributor

I like Ref :]

yorkie
yorkie approved these changes Jan 7, 2017
View reviewed changes
Copy link
Contributor

@yorkie yorkie left a comment

Choose a reason for hiding this comment

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

LGTM, and should we notify all collaborator as this PR is patching COLLABORATOR_GUIDE :)

@targos
Copy link
Member

targos commented Jan 7, 2017

Let's see what people want: http://www.strawpoll.me/12049231

/cc @nodejs/collaborators

Trott
Trott reviewed Jan 8, 2017
View reviewed changes
COLLABORATOR_GUIDE.md
@@ -120,6 +120,7 @@ information regarding the change process:
for an issue, and/or the hash and commit message if the commit fixes
a bug in a previous commit. Multiple `Fixes:` lines may be added if
appropriate.
- A `Refs:` line referencing a URL for any relevant background or links.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Maybe omit or links?

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

@Fishrock123
Copy link
Contributor

I think I may have started the "Refs" thing. To me it made more sense as "References ...x" rather than "Reference ...x".

I'm not sure how much value there is codifying this, would we ever use this in tooling like PR-URL or the commit message?

sam-github
sam-github reviewed Jan 9, 2017
View reviewed changes
CONTRIBUTING.md

```txt
Fixes: https://github.com/nodejs/node/issues/1337
Refs: http://eslint.org/docs/rules/space-in-parens.html
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a second Refs that refers to an Issue or PR, to emphasize that Refs can be used for github links, and also that it can occur multiple times?

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

jasnell pushed a commit that referenced this pull request Jan 10, 2017
@gibfahn @jasnell
doc: document use of Refs: for references
59a3f86 
Standardise on Refs:

PR-URL: #10670
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@jasnell
Copy link
Member

jasnell commented Jan 10, 2017

Landed in 59a3f86

@jasnell jasnell closed this Jan 10, 2017
@gibfahn gibfahn deleted the doc-ref branch January 10, 2017 08:09
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 18, 2017
@gibfahn @italoacasas
doc: document use of Refs: for references
d9739ae 
Standardise on Refs:

PR-URL: nodejs#10670
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 23, 2017
@gibfahn @italoacasas
doc: document use of Refs: for references
eff0ef2 
Standardise on Refs:

PR-URL: nodejs#10670
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 24, 2017
@gibfahn @italoacasas
doc: document use of Refs: for references
56eb907 
Standardise on Refs:

PR-URL: nodejs#10670
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@italoacasas italoacasas mentioned this pull request Jan 25, 2017
Closed
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 27, 2017
@gibfahn @italoacasas
doc: document use of Refs: for references
f81bd48 
Standardise on Refs:

PR-URL: nodejs#10670
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@italoacasas italoacasas mentioned this pull request Jan 29, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Mar 7, 2017
@gibfahn @MylesBorins
doc: document use of Refs: for references
48712ac 
Standardise on Refs:

PR-URL: #10670
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
@gibfahn @MylesBorins
doc: document use of Refs: for references
f2ddc72 
Standardise on Refs:

PR-URL: #10670
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Mar 9, 2017
Merged
@aqrln aqrln mentioned this pull request Mar 28, 2017
Closed
2 tasks
gibfahn added a commit to gibfahn/node-review that referenced this pull request Mar 28, 2017
@gibfahn
Use Refs instead of Ref
7edb7cc 
This was standardised in nodejs/node#10670.

Refs: nodejs/node#12040
@gibfahn gibfahn mentioned this pull request Mar 28, 2017
Merged
evanlucas pushed a commit to nodejs/node-review that referenced this pull request Mar 29, 2017
@gibfahn @evanlucas
Use Refs instead of Ref (#9)
9b5b27b 
This was standardised in nodejs/node#10670.

Refs: nodejs/node#12040
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Trott Trott Trott left review comments

@joyeecheung joyeecheung joyeecheung approved these changes

@yorkie yorkie yorkie approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

@sam-github sam-github sam-github approved these changes

Assignees
No one assigned
Labels
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.