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

2020 Node.js Contributors Survey Results #882

Merged
merged 1 commit into from
Jun 17, 2020

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Jun 14, 2020

@nodejs/tsc @nodejs/collaborators ... the compiled results of the 2020 Contributor's Survey.

@ronag
Copy link
Member

ronag commented Jun 14, 2020

That was some interesting reading. Seems to me like there are a lot of mentions of the contribution & review process.

I'm rather new to the organization so I wonder these results usually are used? i.e. do they lead directly to any actionable items and/or issues or are they used more as reference for ongoing and future discussions?

EDIT: I found the answer #865

@addaleax
Copy link
Member

So … yup, this is definitely very interesting. What’s a good place to discuss the results? Here, in this PR?

One thing that comes up quite a bit is being annoyed with C++, which is more than understandable. That was obviously my main motivation behind adding src/README.md, but it seems like that’s either not enough or not what people are looking for, or it’s not discoverable enough. I’m wondering if any @nodejs/collaborators could expand on that, or would be willing to chat about it with me (or others)? I know I do a lot of C++ work, so I do feel that I have a responsible to make that work as accessible to others as possible.

@ronag
Copy link
Member

ronag commented Jun 14, 2020

One thing that comes up quite a bit is being annoyed with C++, which is more than understandable.

I've got around 14 years of experience with C++ from working on https://github.com/CasparCG/server (Windows). However, I have not touched C++ in NodeJS core at all. Not sure if/how I can be of help but if you need someone to experiment with in how to make the C++ parts more approachable I'd be happy to contribute.

@Qard
Copy link
Member

Qard commented Jun 15, 2020

One thing that comes up quite a bit is being annoyed with C++, which is more than understandable. That was obviously my main motivation behind adding src/README.md, but it seems like that’s either not enough or not what people are looking for, or it’s not discoverable enough. I’m wondering if any @nodejs/collaborators could expand on that, or would be willing to chat about it with me (or others)? I know I do a lot of C++ work, so I do feel that I have a responsible to make that work as accessible to others as possible.

I think src/README.md is an excellent resource but, at least for me, I've found it challenging figuring out where exactly things are in the source files, what the class hierarchy is, and awareness of helpers or "better" ways to do things such as using MaybeStackBuffer rather than std::vector. I learn about a lot of these things from suggestions in code review, which is fine, but it does mean more iterations which can overwhelm new contributors. I go in expecting a certain level of iteration because I've been in the issue tracker long enough to have set my expectations but a new contributor might not be expecting so much iteration and interpret it negatively.

@Flarna
Copy link
Member

Flarna commented Jun 15, 2020

Regarding C++: src/README.md provides a good overview/starting point for people able to speak C++. It's really good as the v8 API itself is not that easy to use/understand and the docs there are not that helpful.

I'm old enough that I grow up with C and later C++. During the last years I worked with quite some younger people - mostly fresh from university. They did great work JavaScript/Java/... and associated toolings but once I confronted them with tasks in C++ area there were lost.

So I fear the issue is more the language C++ itself not how it is used by node.

What I miss is some more description about the interworking between JS/C++ which are not that obvious. For example the various places where AliasedBuffer/... are used. It's not that easy to track down all cases where C++ influences JS as you can't just follow a call tree.

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mmarchini
Copy link
Contributor

Ultra-minor nit on formatting:

  • "There are a few individuals that are extremely noisy and write complex comments to grasp. Their language is sophisticated and they are hard to follow for non-native english speakers.
  • They are making Node.js seem an exclusive club of people who have a lot of time to contribute."

Seems like those two items should be only one:

  • "There are a few individuals that are extremely noisy and write complex comments to grasp. Their language is sophisticated and they are hard to follow for non-native english speakers. They are making Node.js seem an exclusive club of people who have a lot of time to contribute."

@jasnell
Copy link
Member Author

jasnell commented Jun 17, 2020

@mmarchini ... I split those intentionally so the second point wouldn't get lost as it's an important point on it's own.

@jasnell
Copy link
Member Author

jasnell commented Jun 17, 2020

Going to merge this but I don't want to discourage further discussion and review

@jasnell jasnell merged commit 6ac3505 into master Jun 17, 2020
@jasnell jasnell deleted the 2020-contributor-survey-results branch June 17, 2020 18:26
jasnell added a commit to jasnell/node that referenced this pull request Jun 18, 2020
The [Contributor's Survey
results](nodejs/TSC#882) highlight the fact that
it is often not easy for contributors to know who the right people are
to talk to about a proposed change or who to ask for reviews of a given
subsystem. We briefly toyed around with codeowners before when GitHub
introduced it but just as quickly disabled it because the structure of
our repository made it exceedingly difficult to get right.

Rather than start with a codeowners for the entire project, I propose
that we start with a small experiment focused on specific subsystems.
Specifically, codeowners for modules, streams, net/tls, http/http2, and
quic (once that lands). We can expand out from there as we see how
things go with the minimal starter set. Initially this just enables
codeowners for the `quic` subsystem.

A couple of points:

1. A codeowner should always be a team, never an individual person
2. Each codeowner team should contain at least one TSC member (to
provide coverage for signing off on semver-major changes)
3. PRs touching any code with a codeowner must be signed off by at least
one person on the codeowner team
mmarchini pushed a commit to nodejs/node that referenced this pull request Jun 20, 2020
The [Contributor's Survey
results](nodejs/TSC#882) highlight the fact that
it is often not easy for contributors to know who the right people are
to talk to about a proposed change or who to ask for reviews of a given
subsystem. We briefly toyed around with codeowners before when GitHub
introduced it but just as quickly disabled it because the structure of
our repository made it exceedingly difficult to get right.

Rather than start with a codeowners for the entire project, I propose
that we start with a small experiment focused on specific subsystems.
Specifically, codeowners for modules, streams, net/tls, http/http2, and
quic (once that lands). We can expand out from there as we see how
things go with the minimal starter set. Initially this just enables
codeowners for the `quic` subsystem.

A couple of points:

1. A codeowner should always be a team, never an individual person
2. Each codeowner team should contain at least one TSC member (to
provide coverage for signing off on semver-major changes)
3. PRs touching any code with a codeowner must be signed off by at least
one person on the codeowner team

PR-URL: #33895
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@bnb
Copy link
Contributor

bnb commented Jun 27, 2020

In the future, perhaps this could live in nodejs/admin ❤️

codebytere pushed a commit to nodejs/node that referenced this pull request Jun 27, 2020
The [Contributor's Survey
results](nodejs/TSC#882) highlight the fact that
it is often not easy for contributors to know who the right people are
to talk to about a proposed change or who to ask for reviews of a given
subsystem. We briefly toyed around with codeowners before when GitHub
introduced it but just as quickly disabled it because the structure of
our repository made it exceedingly difficult to get right.

Rather than start with a codeowners for the entire project, I propose
that we start with a small experiment focused on specific subsystems.
Specifically, codeowners for modules, streams, net/tls, http/http2, and
quic (once that lands). We can expand out from there as we see how
things go with the minimal starter set. Initially this just enables
codeowners for the `quic` subsystem.

A couple of points:

1. A codeowner should always be a team, never an individual person
2. Each codeowner team should contain at least one TSC member (to
provide coverage for signing off on semver-major changes)
3. PRs touching any code with a codeowner must be signed off by at least
one person on the codeowner team

PR-URL: #33895
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
codebytere pushed a commit to nodejs/node that referenced this pull request Jun 30, 2020
The [Contributor's Survey
results](nodejs/TSC#882) highlight the fact that
it is often not easy for contributors to know who the right people are
to talk to about a proposed change or who to ask for reviews of a given
subsystem. We briefly toyed around with codeowners before when GitHub
introduced it but just as quickly disabled it because the structure of
our repository made it exceedingly difficult to get right.

Rather than start with a codeowners for the entire project, I propose
that we start with a small experiment focused on specific subsystems.
Specifically, codeowners for modules, streams, net/tls, http/http2, and
quic (once that lands). We can expand out from there as we see how
things go with the minimal starter set. Initially this just enables
codeowners for the `quic` subsystem.

A couple of points:

1. A codeowner should always be a team, never an individual person
2. Each codeowner team should contain at least one TSC member (to
provide coverage for signing off on semver-major changes)
3. PRs touching any code with a codeowner must be signed off by at least
one person on the codeowner team

PR-URL: #33895
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
codebytere pushed a commit to nodejs/node that referenced this pull request Jul 10, 2020
The [Contributor's Survey
results](nodejs/TSC#882) highlight the fact that
it is often not easy for contributors to know who the right people are
to talk to about a proposed change or who to ask for reviews of a given
subsystem. We briefly toyed around with codeowners before when GitHub
introduced it but just as quickly disabled it because the structure of
our repository made it exceedingly difficult to get right.

Rather than start with a codeowners for the entire project, I propose
that we start with a small experiment focused on specific subsystems.
Specifically, codeowners for modules, streams, net/tls, http/http2, and
quic (once that lands). We can expand out from there as we see how
things go with the minimal starter set. Initially this just enables
codeowners for the `quic` subsystem.

A couple of points:

1. A codeowner should always be a team, never an individual person
2. Each codeowner team should contain at least one TSC member (to
provide coverage for signing off on semver-major changes)
3. PRs touching any code with a codeowner must be signed off by at least
one person on the codeowner team

PR-URL: #33895
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
codebytere pushed a commit to nodejs/node that referenced this pull request Jul 12, 2020
The [Contributor's Survey
results](nodejs/TSC#882) highlight the fact that
it is often not easy for contributors to know who the right people are
to talk to about a proposed change or who to ask for reviews of a given
subsystem. We briefly toyed around with codeowners before when GitHub
introduced it but just as quickly disabled it because the structure of
our repository made it exceedingly difficult to get right.

Rather than start with a codeowners for the entire project, I propose
that we start with a small experiment focused on specific subsystems.
Specifically, codeowners for modules, streams, net/tls, http/http2, and
quic (once that lands). We can expand out from there as we see how
things go with the minimal starter set. Initially this just enables
codeowners for the `quic` subsystem.

A couple of points:

1. A codeowner should always be a team, never an individual person
2. Each codeowner team should contain at least one TSC member (to
provide coverage for signing off on semver-major changes)
3. PRs touching any code with a codeowner must be signed off by at least
one person on the codeowner team

PR-URL: #33895
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
codebytere pushed a commit to nodejs/node that referenced this pull request Jul 14, 2020
The [Contributor's Survey
results](nodejs/TSC#882) highlight the fact that
it is often not easy for contributors to know who the right people are
to talk to about a proposed change or who to ask for reviews of a given
subsystem. We briefly toyed around with codeowners before when GitHub
introduced it but just as quickly disabled it because the structure of
our repository made it exceedingly difficult to get right.

Rather than start with a codeowners for the entire project, I propose
that we start with a small experiment focused on specific subsystems.
Specifically, codeowners for modules, streams, net/tls, http/http2, and
quic (once that lands). We can expand out from there as we see how
things go with the minimal starter set. Initially this just enables
codeowners for the `quic` subsystem.

A couple of points:

1. A codeowner should always be a team, never an individual person
2. Each codeowner team should contain at least one TSC member (to
provide coverage for signing off on semver-major changes)
3. PRs touching any code with a codeowner must be signed off by at least
one person on the codeowner team

PR-URL: #33895
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Trott pushed a commit to addaleax/node that referenced this pull request Aug 18, 2020
As the [2020 Node.js Contributors Survey][] has shown, the waiting
time for pull requests is a non-trivial obstacle to meaningfully
contributing to Node.js.

To reduce that friction, this PR:

- Drops the 48-hour/7-day waiting times as strict rules.
- Drops the `fast-track` label, which is now the implied default.
- Adds a `wait-for-feedback` label that collaborators can add if they
  think that a pull request *should* remain open for at least 48 hours.
- Reduces the strict requirement for merging something to 1 approval,
  and keep 2 approvals as a strong suggestion.
- Allows immediate reverting of pull requests that have been
  fast-tracked, if deemed necessary.

[2020 Node.js Contributors Survey]: nodejs/TSC#882

Refs: nodejs/TSC#882
Fixes: nodejs#33627
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.