Skip to content

guide: use github name instead of name + addr#3052

Closed
yorkie wants to merge 1 commit intonodejs:masterfrom
yorkie:patch-2
Closed

guide: use github name instead of name + addr#3052
yorkie wants to merge 1 commit intonodejs:masterfrom
yorkie:patch-2

Conversation

@yorkie
Copy link
Copy Markdown
Contributor

@yorkie yorkie commented Sep 24, 2015

Just a proposal :-)

@Fishrock123 Fishrock123 added the doc Issues and PRs related to the documentations. label Sep 24, 2015
@mscdex
Copy link
Copy Markdown
Contributor

mscdex commented Sep 24, 2015

Why?

@yorkie
Copy link
Copy Markdown
Contributor Author

yorkie commented Sep 24, 2015

Actually I'm using Node.js COLLABORATOR_GUIDE in one of my business team, and it works great. But I just found we are using Github, right? So sometimes we could directly call someone via Github comments under the commit not via their emails, then the reviewer would automatically get notification.

Just feel it works better than before in that team, so I hope give a feedback to the upstream, Node.js community, never mind to close this if you think this is impossible to apply :-)

@mscdex
Copy link
Copy Markdown
Contributor

mscdex commented Sep 24, 2015

FWIW I am strongly against using @-style mentions in commit messages just because there is no way to turn off the notifications that you get every time someone pushes a commit (especially in forks) with @yourname in the message. I've already suggested to GitHub support a switch to disable strictly those notifications, but I'm not optimistic about seeing it implemented (at least any time soon)...

@Qard
Copy link
Copy Markdown
Member

Qard commented Sep 24, 2015

I get what you are saying about email noise, but this should only be getting applied after review, before a merge. I don't think it'd actually be that noisy.

I'm indifferent on adding github names, but I'm -1 on removing email.

@yorkie
Copy link
Copy Markdown
Contributor Author

yorkie commented Sep 24, 2015

Ok....closing, thanks

@yorkie yorkie closed this Sep 24, 2015
@yorkie yorkie deleted the patch-2 branch September 24, 2015 22:58
@mscdex
Copy link
Copy Markdown
Contributor

mscdex commented Sep 24, 2015

@Qard Actually, you get a notification anytime anyone pushes that commit to their fork too. I am not 100% sure about the exact circumstances, but I'm speaking on prior experience here. It can get annoying quick...

@yorkie
Copy link
Copy Markdown
Contributor Author

yorkie commented Sep 25, 2015

This is my bad, I tend to use Github name just because sometimes workers at that business team are not that strong Github man, so often notification is fine, too. Obviously this doesn't work great for Node.js reviewers team, thank you again @mscdex :-)

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants