Repository navigation
Process around new contributors in Node core #9091
Description
Hi everyone!
I’ve been trying to gather some feedback on our collaboration process from talks with other collaborators, in particular relatively fresh ones.
So here’s a few things that emerged from that and that I would consider worth mentioning to everybody (and at least as a reminder to myself):
- We are doing a good job at making people feel that contributions to core are a significant accomplishment, including the changes that don’t occur in
lib/orsrc/. That’s awesome! - We should be clearer about what kind of changes are necessary to be performed by the author and what kind of changes can be done when landing. Github’s new distinction between Approved/Changes Required might be helpful, but I think it would be good to clarify the distinction every time, especially as the philosophy around this varies noticeably among the group of collaborators.
- Likewise, there is a wide variance among us when it comes to the tone and role we take when approaching PRs and the effort of outsider contributors. We should generally try take on more of a mentoring than a judging role here, and if you don’t feel that you have the ability to be helpful in a given moment, keep in mind that you don’t have to review and answer to everything asap. Having a large and growing group of collaborators means that the work of guiding people’s efforts can be spread out among us!
- Be flexible. As collaborators we are very familiar with all of the guidelines and formalities mutually agreed upon in the project, but outside contributors often are not. Their valuable effort shouldn’t be lost just because we are unwilling to help. If in doubt, trust the collaborator who lands a PR to fix any obvious issues or agreed-upon nits themselves.
- A lot of our
good-first-contributionissues are not very well-approachable, and we largely lack other proper resources that we can point newcomers to. NodeTodo and the Code + Learn sessions are good steps forward, and they clearly show that there is no scarcity of people who are willing to offer a bit of time and effort.
(And again, just to be very clear: All of this are things where I’d like to see myself improve, too!)
I’m going to close this issue in a few days if no significant discussion around anything comes up; in that case, just regard this as a general announcement and maybe something to take to heart. (Also: If you think this should have been posted somewhere else, please tell me. But this seemed like the best way to reach the wider @nodejs/collaborators group).
Activity
bnoordhuis commentedon Oct 14, 2016
Can you clarify what you mean?
Fishrock123 commentedon Oct 14, 2016
related: #6885
addaleax commentedon Oct 15, 2016
I’ll try. I think it mostly boils down to the two explicit goals of reviewing that we mention in our onboarding doc:
Evaluating PRs in their current state obviously plays an important role in reaching the first goal, and while we usually do a good job at pushing contributions into what we consider a right direction, we don’t always manage to do it in a way that doesn’t make the contributor perceive the corrected issues as their mistakes when we can avoid that, which brings us a lot closer to scaring them off as potential recurring contributors. (Which, at least from my perspective, is something we get from people “succeeding” – feeling that they are in a position to come here and support the project with their work.)
As a concrete example, think of the “Please format the commit message according to the CONTRIBUTING.md.” kind of comment (which, again, I have posted myself from time to time) – of course it’s helpful for us collaborators to have everything in the format we expect, but it can easily leave people with the impression that they are not able to contribute to Node, and not because we expect something from contributors, but because of the way in which we communicate it.
I realize it’s kind of inevitable that people who carry out a lot of reviews also are people who are just used to our processes, and that it takes time to empathize with people who aren’t. But generally, we should be aware that even just coming here can be scaringly unfamiliar (ex: In my own first PR here, I asked whether I should edit the
CHANGELOG.mdmyself as part of the PR, accompanied by a certain fear of just screwing everything up and unnecessarily using up people’s time by asking about it 😄), and we are in a position where we can show that it doesn’t have to be.Also, just to be clear: I’m not saying that anyone should be able to get any change into Node by just trying hard enough. It’s just that it’s pretty silly to push people away over the tone with which we talk to them.
Trott commentedon Oct 15, 2016
@addaleax has already expanded on this quite a bit. I'd like to add an example. Insert standard caveat that I am guilty of these things myself from time to time.
In my opinion, and over-simplifying a bit, it boils down to:
Mentoring = helpful and providing positive direction.
Judging = criticizing without offering a nudge in the right direction.
So, the example:
It's more work, and ideally work the person should have done themselves, but part of being "mentoring" is coaching people when they mess up rather than making them feel foolish.
I also want to add that I think most of the people who are active on the project do a pretty good job of this. But there's also always room for improvement.
EDIT: Also, I am open to the idea that this is really just an idiosyncrasy of mine and most people don't read "We've talked about this before" or whatever the same way I do.
Trott commentedon Oct 15, 2016
(Also, it's hard to avoid "this has been discussed before" when someone drops in the middle of a conversation with something. It's unlikely to be worth it to invest a ton of effort into addressing every rando's comments with a bunch of references. So, you know, context, I guess.)
thefourtheye commentedon Oct 15, 2016
Can we have boilerplate text which would ask the users politely and in a more engaging way?
Personally I would be happier if somebody gave me a document like this, rather than them telling me to fix the mistakes one by one. That might make one feel that the reviewer might be judging.
If we want to make the contributing process easy for the new contributors and as well the collaborators, automating the commit formatting and landing process would be a better option I believe. That way, we would reduce the amount of process related work needed to be done by both the parties.
lpinca commentedon Oct 15, 2016
I think I've used this many times but hopefully in a friendly manner. The reason is that it takes more time for me to reword the commit message than reviewing/landing the whole pr.
Another reason is that I don't like to change the words used by the authors without their consent as it is part of their contribution.
Anyway I'll try to avoid that now.
addaleax commentedon Oct 15, 2016
Yeah… I don’t particularly like that either, and pointing out that the commit message should be updated to conform to certain standards is okay. I would say that it depends on the user – in particular, if it’s obvious that one is dealing with someone who has the necessary experience with git, some short hint might suffice, and I can see why some people would want to receive only that information.
But sometimes it would probably be really helpful to give an example of how a reworded commit message might look like, or maybe even (especially if we come up with some boilerplate messages) a short description of how to amend the commit message.
I think something like gathering canned replies for github that can be modified on a case-by-case basis would be awesome, yes!
bnoordhuis commentedon Oct 15, 2016
How would that work this side of the Singularity? Writing up a good commit log is manual labor, no amount of tooling is going to fix that. If you mean wrapping long lines, I don't think that is what is being discussed here.
Maybe it's a native/non-native speaker thing but I take such comments on face value, I don't read between the lines. It's also how I intend it to be taken when I use the phrase myself.
I don't think I'd use "everyone is on the same page." I can't quite articulate why but it feels subtly wrong. Maybe it's because it presumes to speak for everyone involved when it's highly unlikely that everyone was indeed 100% in agreement, and even more unlikely that I know each person's personal beliefs well enough to speak with authority.
If you can come up with an alternative that has the word 'consensus' in it, I'll take it! :-)
I won't stop you but I'm not going to use them. I hate text templates, they feel impersonal and lazy, and I bet I'm not the only one who feels that way.
Give me a grumpy maintainer any day, at least you know they cared enough to type up a reply!
addaleax commentedon Oct 15, 2016
Oh, yeah, me too – we shouldn’t make it be perceived that way. I think it does depend a lot on how you use them; for example, I have my “author name” template, which has served me pretty well so far:
If you think that that sounds too unpersonal (obviously, the ellipsis is filled out before posting it), please tell me. Generally, this seems to have worked pretty well.
Trott commentedon Oct 15, 2016
On both of those points: I certainly concede that there is a lot of subjectivity on these sorts of things.
I would be very surprised if everyone agreed with my example.
The more I've been thinking about it, and because of all the subjectivity, I kind of wish I hadn't included the specific example and just left what I said in more general terms.
bnoordhuis commentedon Oct 16, 2016
It feels pretty personal actually because you typo'd 'Your'. Nice touch! :-)
Jest aside, it's pretty alright - not the wall of feel-good text that some projects use and makes you question whether you're dealing with a human or a bot - but I still don't see myself using it.
No, that's okay; specificity is good. Even if I disagree 9 out of 10 times, the tenth time it shifts my perspective.
addaleax commentedon Oct 20, 2016
Er, yes. Thanks for pointing that out. ;)
But yeah, that’s about the kind of text I would have in mind – somewhat short texts for specific situations.