Do we usually use non-ASCII characters as quotation marks? There are ASCII quotation marks in lines 69 and 77.

I like to do this, mostly because it does look a lot nicer to me (and partly because I like to know if something breaks because of this – if something can’t handle UTF-8 in 2017, it’s useless). I’ve done this before in the docs, and so far nobody has complained.

That's fine by me, just wanted to check :)

Same here, Unicode while line 112 still sticks to ASCII. Please feel free to ignore this if you feel it is irrelevant :)

+1, this should be consistent

fwiw I don’t see the while line 112 still sticks to ASCII one, are you sure you got the right line?

Line 112: + * You have the power to approve any other collaborator's work.

I think they mean new line 112 - https://github.com/nodejs/node/pull/13413/files#diff-c00e7ffdc40896f8ff6567c07f9eb43cR112

Maybe it would be better to set this only for the local node repo?

I’ve been mentioning manually sometimes that you can set this locally if you really want to… I know I would like to have this as a global option, but sure, if others don’t, I can change this.

I'm good with this as written

I don't think this is universally true - sometimes there is value in having a PR off a branch that others can easily edit.

We have Allow edits from maintainers. for that now, right?

With the ability for collaborators to push into PR branches in forks, is that really an issue tho?

There is an edge case. If the PR is against the fork's master you can't force push a rebased ref set, or if you can you lose the ability to edit.
So maybe:
Continue to always create PRs from branches on your own github fork

There is an edge case. If the PR is against the fork's master you can't force push a rebased ref set, or if you can you lose the ability to edit.

Do you mean PR from a fork’s master? I don’t think we need to worry about that, virtually all collaborators have made more than one PR at the time of their onboarding so they know not to do that.

🤷‍♂️ I'm good anyway

+1 to this, using branches in Node is not usual behaviour, we should start with the standard workflow.

Maybe:

-    * Always continue to PR from your own github fork
+    * Continue to raise PRs from your own github fork

though

the collective?

done!

Maybe we should have an acronyms section? LGTM, PTAL, etc

any another

done!

except certain operations on GitHub comments 😞

I have no suggestions on how to say this succinctly... but it may be worth adding a reminder that deleting the branch for a PR removes the commit history from that PR (that is, the PR will show 0 commits / 0 changes)

Turn it into a positive:
Almost any mistake you could make can be fixed or reverted
IMHO it's good to add but try to learn from your mistakes

IMHO empathy is too vague:
Maybe Being kind and courteous towards users is important

@refack I think "being kind and courteous" is too low a bar. I agree that "empathy" can be hard to define specifically and may not be understood by everyone, but I prefer it to "be nice" which (to me, at least) minimizes things.

Maybe what's needed here is a link on the word "empathy" to another doc (or another part of this doc) that might explain things more fully. Probably out-of-scope for this PR, though. And I'm certainly open to other ideas.

(Also, "be nice to people" is spelled out in the next line, woot!)

I'm happy if we can assume that everyone watched "Star Trek TNG" and knows what an Empath is 😄

Nit (no so sure)
maybe /s/this/there/
or ..., a good place to ask is there.

maybe /s/power/authority/

You can't (GitHub does not allow it) so:
You can't approve your own pull requests, so you'll need at least another Collaborator's approval for you own PRs.

I’ll pick the first part of the suggestion, the rest is kind of implicit, right? (Also, remember that there’s usually an extra audio or text stream that goes over all of this – I’m okay with not explaining everything in here, just the outlines)

👍
I was just imagining someone reading this beforehand, and it was a one of questions I explicitly asked.

Re nodejs/CTC#128 I would elaborate on it "blocking":
When explicitly using `Changes requested` you are effectively blocking the PR, so be courteous and explain why you consider your comment a "blocking issue" - comment...

I think they mean new line 112 - https://github.com/nodejs/node/pull/13413/files#diff-c00e7ffdc40896f8ff6567c07f9eb43cR112

period after et al. (it's in the phrase not an end of sentence period)
or even replace with the more common etc. (also . is part of the word)

IMHO since it's a list Also, is unnecessary.

Turn it into a positive:
Almost any mistake you could make can be fixed or reverted
IMHO it's good to add but try to learn from your mistakes

Take-it-or-leave-it nit: While we're in here, let's drop effectively. It introduces more questions than it answers. Collectively, the Collaborators are the owners of the project. Saying effectively just signals that we don't actually mean it.

There still is effectively vs formally. Isn't the project formerly owned by the "Node.js Foundation"

BTW: 👍 on removing

I like this line a lot.

Micro-nit: Remove the - and make this two sentences (separated with .) or two clauses (separated with ;).

done :) went with ;, that seemed the most natural to me.

Micro-nit: remove just

Less micro: Remove for many of the teams listed there. People should freely ask. If they need to do something to join a WG or whatever, then it can be explained when they ask.

Micro-nit node -> Node.js in two places on this line

Micro-nit: I think we tend to style it capitalized, so collaborators -> Collaborators

This suggests that multiple commit PRs are not ok, which I don't think is the intent.

Maybe:

-    * Make new branches for all commits you make!
+    * Make a new branch for each PR you raise.

[question to a native english speakers]
raise vs create vs request
Intuitively would use raise for Issue, request for PR (since PR = "Pull Request", but turned into a noun)? or is create best?

Not a native English speaker, but my preference would be create, raise, request. Why not submit?

For me:

request sounds weird, you don't request a Pull Request, you request that your changes be landed by raising the Pull Request. It'd be like questioning a question.

I prefer raise to create, it has the feeling of bringing it to people's attention, and it matches raising issues. I'd say raise, create, submit, or open are all fine though.

I went with submit, to my ears raise seems like it matches Issues much better than PRs.

Maybe also link to https://github.com/orgs/nodejs/teams ? I didn't realise that I could now see who was in what team when I was onboarded.

You can’t see that page if you’re not a member, so it’s at least going to be confusing for those who read through the doc a bit in advance.

Looking at this again, I see you linked to the Collaborators and members teams above, so the team listing can probably be deduced for that.

The some people like to comment seems weird to me, the point is that either is fine right?

-        the changes in a pull request using Github’s approval interface
-    * Some people like to comment `LGTM` (“Looks Good To Me”)
+        the changes in a pull request using Github’s approval interface,
+        or by commenting `LGTM` (“Looks Good To Me”).

Either is fine, but by now people are almost exclusively using the approval interface, the only exception I know of being @mscdex (and @mhdawson who usually approves and comments with “LGTM”).

The reason I prefer that method is because github does not automatically remove approvals after new changes are pushed, so having green checkmarks still show after new changes are pushed is not ideal to me. I would rather "risk" having my name left out of the 'reviewed by' list than be erroneously listed as having approved something that I didn't review.

I understand it's ultimately inevitable at the time of landing, as metadata is typically landed last, but I'm mostly concerned about pushes prior to that point.

@mscdex interesting point, but I'm not sure how commenting LGTM is better. If people are using the node-review chrome extension, that'll add your name to the metadata anyway, no matter when you LGTMed.

Sounds like it would be easier to fix in node-review then get that as a feature from GitHub (dismiss approvals on new commits)
On second thought it's not trivial, since new commits could be just nit fixes or benign improvements 🤔

@gibfahn It's mostly for anyone who solely uses the "approved" list for the "reviewed by" list. It's better than nothing.

@refack To me it doesn't matter what the changes are.

It's mostly for anyone who solely uses the "approved" list for the "reviewed by" list. It's better than nothing.

Hmm okay, but doesn't that mean that those people will never record your approval at all?

Ref: nodejs/node-review#12

@gibfahn I already mentioned that I would rather chance not having my name on a PR/commit than have my name listed as having approved something I did not review.