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

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

Closed
wants to merge 1 commit into from

Conversation

yorkie
Copy link
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
Contributor

mscdex commented Sep 24, 2015

Why?

@yorkie
Copy link
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
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
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
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
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
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