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.

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

Closed
wants to merge 1 commit into from

Conversation

gibfahn
Copy link
Member

@gibfahn gibfahn commented Jan 6, 2017

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
@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 :]

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

@@ -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?


```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
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
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
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
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 27, 2017
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
MylesBorins pushed a commit that referenced this pull request Mar 7, 2017
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
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
gibfahn added a commit to gibfahn/node-review that referenced this pull request Mar 28, 2017
evanlucas pushed a commit to nodejs/node-review that referenced this pull request Mar 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.