(By the way: I assume it’s by accident, but you keep opening PRs from the nodejs/node repo instead of your fork … we don’t do that except for releases branches, can you try to make sure you open PRs from your fork for the future?)

(By the way: I assume it’s by accident, but you keep opening PRs from the nodejs/node repo instead of your fork … we don’t do that except for releases branches, can you try to make sure you open PRs from your fork for the future?)

Sorry, apparently it happens when I use the (limited) web interface.

Addressed comments. PTAL.

@refack I dismissed that red cross sign, but not explicitly approving, I'm +0 on this. Generally it looks good but I have several concerns that overweight it.

1. Apparently, a significant amount of the first-time contributors either don't look at this document at all, however prominent it is and despite the link being shown when they open a PR, or just skim through it without really understanding it. Just look at all those 100 characters length capitalized commit messages in PRs with the "commit message follows commit guidelines" item being marked with a tick. I'd really love to see ways and ideas to make this information more accessible, not to overwhelm people with another wall of text.

2. I don't really think all of these points can be qualified as "rules of thumb". Some of them are arguable and really depend on the situation and some of them are not just rules of thumb but rather requirements that are better moved to appropriate places of the document, if there are ones. I don't think there's a section covering the code style, though, and I'd rather create it somewhere and populate it with information about the Google C++ Code Style, cpplint, our custom ESLint rules and how to run the linter, etc.

@aqrln I assumed so. Let's let this PR stew (as PRs do). Anyway I hope this PR is a good start
I'll add you reservations as invitation for ideas in the first comment.

P.S. This is actually a fix to #12636

I'd rather see less text, not more, in the CONTRIBUTING.md. I'd prefer a PR that removes everything that isn't absolutely essential.

If we're going to provide helpful hints like this, I'd prefer they be below the instructions for forking etc., rather than at the top of the doc.

I think most people will be discouraged by a wall of a text and won't read it. If we want to include this content, it should be one sentence per point with links to other docs if further explanation should be required.

After @aqrln's @Trott's comments about "wall of text", I collapsed the explanations, so now it's a consise self-contained "rules" list

But IMHO even before it wasn't so bad...

I'd rather see less text, not more, in the CONTRIBUTING.md. I'd prefer a PR that removes everything that isn't absolutely essential.

If we're going to provide helpful hints like this, I'd prefer they be below the instructions for forking etc., rather than at the top of the doc.

I think most people will be discouraged by a wall of a text and won't read it. If we want to include this content, it should be one sentence per point with links to other docs if further explanation should be required.

I agree on less text but would go the other way, leave this guideline, and split technical stuff to another file. IMHO People should think a little bit beforehand, they can figure out the technical stuff later. (yeah, I heard that too. Funny that it is coming from me ☺️)

@lpinca @Trott fixed your comments.
Have you any idea for a better place for this or is #12748 a good direction.

Have you any idea for a better place for this or is #12748 a good direction.

@refack I don't know what problem this is trying to solve. To the extent that I am aware of problems with the CONTRIBUTING.md doc, this would seem to exacerbate them.

This doc is mostly for new or infrequent contributors. While there are exceptions, the usual problem we have with these types of contributions is not (for example) that the change set lacks an obvious motivation. Yes, it happens, but it's not the biggest or most obvious pain point.

The problems we see frequently stem from the fact that people either don't read it or don't understand this document. So, for example, we get commits that don't adhere to the commit message guidelines. If we're trying to solve that problem (which would be laudable), I would say the solution is to make the material in the CONTRIBUTING.md more succinct and more scannable. Perhaps we can make it more visual (although there may be problems with that too).

I don't see any Collaborators (or anyone else for that matter) agreeing that either these changes or #12748 are needed or a good idea. I'm not even seeing positive GitHub reaction emoji things for either of them. I think the whole approach needs a re-think.

Documentation is difficult (and the difficulty is greatly exacerbated by people thinking it's not difficult but that's a long comment for another day). I don't have any concrete suggestions as to how to make the doc more concise and scannable, but that's what I'd focus on instead. (Oh, here's a concrete idea: Maybe a short table of contents or set of internal links near the top, just below the Code of Conduct text: If you're opening an issue, go here; if you're trying to contribute code, go here. Might help the doc feel less overwhelmingly long. Maybe. Just an idea.)

/cc @nodejs/documentation for some contrary/varied/other/concurring opinions/ideas

@Trott I wasn't sure either that CONTRIBUTING.md is the right place for this, but IMHO these things should be put down in writing.

My motivation is that these "rules of thumb" or "best practices" are generally agreed upon, but the knowledge is not transferred or easily findable to new/infrequent contributors (in the wide sense not the @ nodejs/collaborators sense). I've refed some PRs that didn't consider these "rules" and so had to be addressed ad-hoc.

I agree the general mood for the PR is ambiguous, but the participation and commenting was fairly extensive, so I figure people agree with what's written, or in the least care 👍

Added help-wanted and title to first comment.

@Trott I'll stop editing this PR (it's on a bad branch anyway)
I moved the (well edited) content to #12791 which adds a new "catch all" guide.

I'm still looking for help in how to get the intended audience to read it.

2. would prefer we avoided the term "rule of thumb"... Check out the
   Wikipedia article, but there is a chance some people may get the wrong idea

Ohh dear.
The current mood is to call it "Best Practices"

1. maybe a bunch of this content would make a good blog post for the nodejs
   medium

@MylesBorins who edits the medium? I could turn this list into a "how to submit your first PR" or "how to submit a good PR" post for editing.

@refack you should make a post over in @nodejs/evangelism