-
Notifications
You must be signed in to change notification settings - Fork 162
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
Update Contributor Covenant Code of Conduct to v2.1 #851
base: main
Are you sure you want to change the base?
Conversation
@ljharb: My understanding is that this PR is split in three commits, one of which adopts the line breaks and indents of the upstream version in order to make it easier to track upstream. That seems like a reasonable choice. And if we decide not to track it, I think it makes more sense to keep to whatever indenting we had (and remove that commit), rather than come up with yet another version by editing this PR further. So I suggest marking all of those change suggestions as resolved without committing them. |
ah, i didn't read clearly enough i guess. Making it easier to track upstream is nice, but yes, I agree - all of my edits should be theoretically reverting the hard-wrapping. In general, hard wraps make text less legible and maintainable, so I'd prefer to soft-wrap everything even if it deviates from upstream. |
As mentioned in Slack, @Relequestual kindly split up his PR in three commits, one of which shows the changes between Contributor Covenant 2.0 and 2.1, one which fixes links, and the last one fixes indenting and line breaks to match the upstream version. It turns out that the changes between 2.0 and 2.1 are literally two words that add "caste" and "color" to protected categories. I see no reason not to move forward with this update for now, noting that we want a lightweight announcement to project maintainers about the upcoming changes. |
I don't have an opinion either way, but clearly those changes in the same PR are making adoption of this minor revision much more complicated than necessary. Let's keep original indentation and trailing whitespace to avoid creating confusion. @Relequestual would you be willing to rebase your changes keeping only 5b5df2b and the non-whitespace changes in 52852c4? |
I'm fine with removing trailing whitespace, to be clear; it's the line wrapping changes i'm not happy about. |
This is a PR that a large number of folks in the community will need to have a look at, the easier we make their lives by avoiding unnecessary distractions, the better. |
This PR, sure - as with any future updates. One way to make the diffs simple is to match upstream formatting. Another is to reformat the replacement as part of the PR. Since I prefer the current formatting, I'd prefer the latter approach. |
tl;dr: I don't want to bikeshed. I'll revise.
I hadn't anticipated matching upstream formatting to be a controversial change.
Personal preference does have its place, but I had to wonder, why did CC-CoC have a hard wrap? I found an answer: EthicalSource/contributor_covenant#757 (comment)
Well, point 2 isn't accurate. Not all licences (including the one used here) use a hard-wrap. Whatever people feel is best, I don't want bikeshedding to hold up this PR. |
Note: Remove "project" in updating to 2.1, but was actually a later re-publish of 2.0. See openjs-foundation#852 Also auto removed trailing white space on save. Ignore white space to diff
I force pushed as a single commit. |
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.
Given the versioning issue identified in #852, it would be good to include a link in the PR description to the exact upstream commit from which the CoC text was copied.
@@ -111,11 +112,11 @@ Community leaders will follow these Community Impact Guidelines in determining t | |||
|
|||
**Community Impact**: Demonstrating a pattern of violation of community standards, including sustained inappropriate behavior, harassment of an individual, or aggression toward or disparagement of classes of individuals. | |||
|
|||
**Consequence**: A permanent ban from any sort of public interaction within the project community. | |||
**Consequence**: A permanent ban from any sort of public interaction within the community. |
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 believe that this would be a highly contentious change.
The concern, expressed by a number of maintainers in the past, is that a project could be forced by the Foundation or the CPC, to ban a contributor for their actions in a different project, without having had any transparency or say in the CoC enforcement process.
I suggest leaving this change out of this particular PR (as this would be reverting an edit made to 2.0, not updating 2.0 to 2.1) and discussing the consistency issue as part of #852.
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.
That's not what this means though. "community" doesn't mean it must be the entire community, it just means it may be. With "project", it must not be.
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.
IMO community
reads as must and project community
reads as must not.
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.
hmm - to me, it would require a qualifier for that, because "community" is a vague, undefined term.
What about:
**Consequence**: A permanent ban from any sort of public interaction within the community. | |
**Consequence**: A permanent ban from any sort of public interaction within the community (the project, and potentially more widely). |
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 don't think this is the point in the doc where we should be clarifying the term "community". It's also referred to elsewhere, e.g. in the "Temporary Ban" section:
Consequence: A temporary ban from any sort of interaction or public communication with the community for a specified period of time.
To be clear, that bit is already now in our CoC.
I think the issue here is that as with our CLA, we're using an upstream text that presumes a single "project" or a single "community" as the user; we instead have the wider OpenJS Foundation "project"/"community" as an umbrella for individual named projects.
So really this upstream change isn't the issue; the issue is that we do not include a "community" definition in e.g. a preamble to this document. And that's a problem that's independent of the Contributor Covenant version we're using.
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.
This is not the case. There is more than one "version 2.0". See #852 (comment)
Oof! I hadn't properly registered this. Apologies and thanks for catching it!
Unfortunately, I don't think that changes the fact that some projects will have an issue with this edit.
I think our reasonable options are:
- Don't upgrade for now (and allow JSON Schema to run Contributor Covenant 2.1 until we sort this out), or
- Update to Contributor Covenant 2.1, but keep permanent bans scoped to the project's community, or
- First agree on a definition of the meaning and scope of community in Change to Contributor Covenant Code of Conduct are inconsistent #852 that allows us to update to the latest version of Contributor Covenant 2.0 (EthicalSource/contributor_covenant@a63f697), and then update to Contributor Covenant 2.1.
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 don't think options 1 and 2 are viable, given that this discussion has identified the need to define "community" in this context. And as I point out above, this also matters for temporary bans, which even now target the whole "community", without a "project" qualifier.
My sense of prior discussions on this has been that we consider "community" to here mean the project's community. But let's talk about this at the CPC meeting tomorrow?
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.
My sense of prior discussions on this has been that we consider "community" hear to mean the project's community. But let's talk about this at the CPC meeting tomorrow?
yes and yes
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.
Was there a resolution at the meeting?
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.
Sorry, scratch that, I see @jorydotcom's explanation. Seems like this conversation could be marked resolved.
@Relequestual wrote:
Thanks Ben, that really helped untangle the mess here. |
It looks like there was no consensus on the CPC meeting #864 What's the next step for JSON Schema? Wait I guess? |
The JSON Schema CoC uses Contributor Covenant-CoC 2.1 (Commit tagged 8a3be13 - Noting because of previous semver issue).
@eemeli suggested making a PR to update the OpenJSF CoC to use 2.1 as the change is minimal.
This update adds two words and removes one to the CoC, and updates all numerical references to CC-CoC.
This PR was previously in three parts
(It's best if you diff locally using
git-diff --word-diff
)(The second commit also re-adds "project community" back to permanent ban consequences, which is a deviation from the original in 2.0 here.)
-w
or ignore white space enabled on GitHub)Rebase after accepted: #848