-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
improve discovery of CoC #3774
improve discovery of CoC #3774
Conversation
Oh that is an improvement. I actually struggled to find it when I went to it for reference a couple months ago. |
LGTM |
LGTM but the commit log should follow the guidelines from CONTRIBUTING.md. :-) |
wanted to make sure it was all good before finalizing @bnoordhuis, amending the commit now. |
hoping this commit fits the guidelines. as a first time contributor here, i do think the instructions are a bit confusing. (perhaps that will be my next contribution!) r? @bnoordhuis |
I'd use I agree the current CONTRIBUTING.md is not a paragon of clarity. |
LGTM |
i can just amend now @bnoordhuis, that portion of the contributing is also very confusing as the topic wasn't clear to me. i just assumed that the label this PR was given is what should be used. |
This is effectively a duplicate of #3722 .. I'll let you and @thealphanerd choose whichever one remains. |
(It totally is.) @ashleygwilliams May I suggest discussing in relation to #3726? |
For reference, |
eee, @Fishrock123 , i should have checked. my bad. and re: the guidelines i'll czech @3726 and yeah i tend to agree re: /root files ... is doc the right name tho? is there an issue for this? |
|
More than happy to close my pr and move forward with this one. Mine did not
|
* Avoid the use of personal pronouns in code comments or | ||
documentation. There is no need to address persons when explaining | ||
code (e.g. "When the developer"). | ||
[Please read it.](./CODE_OF_CONDUCT.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the reason the CoC was originally included in CONTRIBUTING.md
is that this file is automatically linked by GitHub on the "open a PR" form. I might be imagining this, but I think there was some legal reason that that started happening, too, related to the developer's certificate of origin. Given that, it might be good to note to the readers coming in from that link that the CoC is a binding part of contributing to Node:
By submitting a pull request to, issue to, or commenting on a pull request or issue on any
of the "nodejs/" repositories, or using the #node.js or #io.js IRC channels, you agree to
abide by the Node.js [Code of Conduct](./CODE_OF_CONDUCT.md).
I'd rather not hold up the PR for that, though — it can be done separately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be imagining this, but I think there was some legal reason that that started happening, too, related to the developer's certificate of origin
Correct. The DCO must be in the Contributing.md file because it gets linked to the first time you go to post a PR to a repo on GitHub. Similarly, we increase the likelyhood that someone will see the CoC if it is in that file for the same reason (although a link may be enough, i'm not the expert on these things).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Who do we talk to about determining if the link is enough?
If the link is not enough, maybe we could post it in full in CONTRIBUTING.md and in the separate file for linking purposes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm happy to dump it in contributing as well for now til we shake out legal. seem ok? say the word and i'll amend
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear, there's no legal requirement on the CoC being in a specific place (only the DCO). The only consideration here is "how do we make it more likely they will find it." I don't know entirely how to answer that, my gut is that a link at the top of CONTRIBUTING might be found more easily than a large amount of text at the bottom, but i have no real data to back that up and i'm not an expert on where new contributors find information :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh yeah, duh. 💯 @mikeal i'll put it up at the top now.
@Fishrock123 on moving the other top level files:
|
h'ok, moved the CoC mention to the top of |
LGTM. |
LGTM unless we need to do something more for the contribution guide legally. |
i like squashing @jasnell. uno momento, then ready for merge |
- move CoC from CONTRIBUTING to top-level, separate COC file - add note/link in CONTRIBUTING - add note/link in README (both at top, and newcomer resources) - move CoC section in CONTRIBUTING to top of file
LGTM! Thanks for addressing my comment! |
LGTM |
@ashleygwilliams Just a note, there's a whitespace error I saw while applying this, could you please set |
I like this -- makes it easier to find. Some suggestions inline. |
@Fishrock123 Get your point. In the name of progress lets skip my feedback. |
Let's limit the scope of feedback to matters that make the CoC more discoverable and refrain from considering comments about its content :) |
agreed @mikeal. am happy to start another issue for that discussion, though i think we may need to appeal to higher authorities for actual changes. it may also be a nice thing to sandbox in the inclusivity-wg: https://github.com/nodejs/inclusivity (in case you didn't know it existed @jbergstroem ) |
👍 @mikeal ... I can get this landed. @ashleygwilliams unless you'd like to do so, I can make the s/TC/TSC replacement (#3774 (comment)) in the text when the commit is actually landed. |
+1 for landing. |
- move CoC from CONTRIBUTING to top-level, separate COC file - add note/link in CONTRIBUTING - add note/link in README (both at top, and newcomer resources) - move CoC section in CONTRIBUTING to top of file PR-URL: #3774 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
Landed in d781cdc |
Just as a side note: I've submitted a feature request to Github suggesting that CODE_OF_CONDUCT.md files contained in the root of a project be promoted in the UI similar to how README.md and CONTRIBUTING.md files are promoted. Obviously we'll have to wait to see if they actually do something with the suggestion ;-) |
@jasnell +1, good idea |
- move CoC from CONTRIBUTING to top-level, separate COC file - add note/link in CONTRIBUTING - add note/link in README (both at top, and newcomer resources) - move CoC section in CONTRIBUTING to top of file PR-URL: #3774 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
- move CoC from CONTRIBUTING to top-level, separate COC file - add note/link in CONTRIBUTING - add note/link in README (both at top, and newcomer resources) - move CoC section in CONTRIBUTING to top of file PR-URL: #3774 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
landed in v4.x-staging as 643d949 |
- move CoC from CONTRIBUTING to top-level, separate COC file - add note/link in CONTRIBUTING - add note/link in README (both at top, and newcomer resources) - move CoC section in CONTRIBUTING to top of file PR-URL: #3774 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
- move CoC from CONTRIBUTING to top-level, separate COC file - add note/link in CONTRIBUTING - add note/link in README (both at top, and newcomer resources) - move CoC section in CONTRIBUTING to top of file PR-URL: #3774 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
- move CoC from CONTRIBUTING to top-level, separate COC file - add note/link in CONTRIBUTING - add note/link in README (both at top, and newcomer resources) - move CoC section in CONTRIBUTING to top of file PR-URL: #3774 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
Original commit log for libuv@e900006 follows: thread: add support for affinity (nodejs#3774) Backported thread affinity feature and related dependency commits from master. It will add support for those APIs: uv_cpumask_size, uv_thread_setaffinity, uv_thread_getaffinity. The supported platforms are Linux, Freebsd, and Windows. Empty implementations (returning UV_ENOTSUP) on non-supported platforms (such as OS X and AIX). Original commit log for libuv@64669fd follows: thread: add uv_thread_getcpu() (nodejs#3803) Add uv_thread_getcpu() api to get the cpu number on which the calling thread is running.
as we all know, many do not read
CONTRIBUTING.md
. given that the COC was buried at the bottom of that, i figured that moving the COC to a top level file, and then mentioning it in both theREADME.md
and theCONTRIBUTING.md
would make it hard to miss 😉