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

x/build/devapp: flag new contributors in CL list #22154

Open
kevinburke opened this issue Oct 5, 2017 · 27 comments
Open

x/build/devapp: flag new contributors in CL list #22154

kevinburke opened this issue Oct 5, 2017 · 27 comments
Labels
Builders x/build issues (builders, bots, dashboards) Proposal Proposal-Accepted
Milestone

Comments

@kevinburke
Copy link
Contributor

I was speaking with someone today who mentioned that they read some of the CL's and issue responses and felt intimidated. Specifically they were worried that they would submit a CL and there would be a mistake with it and they'd get unfriendly feedback.

It's easy to see how this could happen. If I make a dumb mistake it's probably due to carelessness and it's worth calling out, where if a new contributor makes a mistake, it's probably due to not knowing the context or the build tools well enough. But that can be hard for a reviewer to see, or a reviewer might not know to switch modes before reviewing a CL.

We have the "Thanks for your first contribution" commit message, but maybe we could add something else to flag for reviewers that the contributor is new to the project? Maybe gopherbot could add a "New-Contributor: +1" label to CL's where the submitter has submitted less than five CL's. Or a gopherbot message that says e.g. "Note to reviewers: This CL was submitted by someone new to the Go project! They might not have all of the context that others have."

@gopherbot gopherbot added this to the Proposal milestone Oct 5, 2017
@kevinburke
Copy link
Contributor Author

This isn't to say that there aren't other things we can do to correct this perception, or to correct the actual problem.

@davecheney
Copy link
Contributor

davecheney commented Oct 5, 2017 via email

@kevinburke
Copy link
Contributor Author

kevinburke commented Oct 5, 2017

As far as I understood, they got pointed at the "Help Wanted" issues during a meetup, read some of the comments and CL's, and got worried that if they submitted something, it would get nitpicked, or they'd submit something with a mistake and then get judged harshly. I think they got this impression from reading communication between longtime contributors. I don't want to badger this person about the specifics of what conversation they read. Sometimes I think Go contributors can be terse in their communications with each other that might be intimidating to new people, if they expect that person will communicate with them in the same way.

I'm sensitive to this issue, but I don't know, even if we did a fantastic job of being open and welcoming, I think it's natural to be nervous before submitting your first CL, the same reasons people get nervous before speaking in public, starting a new job, etc.

@ianlancetaylor
Copy link
Contributor

It's always hard to take your first step into a new arena, especially a public one. I wonder if your suggestions are too focused on ensuring that their first step doesn't stumble; that is certainly worthwhile, but it only helps people who have already taken that first step. How do we encourage them to take the step in the first place?

@kevinburke
Copy link
Contributor Author

kevinburke commented Oct 5, 2017

It might be good to keep this thread focused on this specific proposal, but I'm giving a talk about your question tomorrow at Gotham Go. I think the goal is to lower the bar as much as possible, e.g. encourage more use of the "scratch" repo for getting familiar with Gerrit, and encourage extremely changes like adding "s" to links so people get familiar with the tools and build trust in the system.

@davecheney
Copy link
Contributor

davecheney commented Oct 5, 2017 via email

@kevinburke
Copy link
Contributor Author

My talk tomorrow is going to go over that in detail, my comments in this thread are in response to the specific feedback that I got from a contributor, and a proposal for a possible improvement.

@as
Copy link
Contributor

as commented Oct 5, 2017

There is a growing body of social-science research that may contribute useful information toward the underlying goal of the proposal:

https://www.researchgate.net/profile/Beatrice_Hasler2/publication/257995020_Online_Intergroup_Contact/links/0deec526913b52de52000000.pdf

@ggriffiths
Copy link
Contributor

ggriffiths commented Oct 5, 2017

I can give my two cents about this as a new contributor.

When submitting my first CL earlier this week, I wasn't aware that I would get the "Congratulations on opening your first change" message. Similarly, I'm not sure new contributors would know that their first CL would be marked as "New-Contributor: +1", given that they aren't as familiar with the process to begin with. This means that a new contributor might not be more inclined to take that first step due to this label. Perhaps if someone were to look through all the CLs and see the label a few times, they would be more comfortable making a CL.

Regarding the actual review process, I can see how it would indicate for reviewers to be kind about silly mistakes, tests failing, or contribution process mistakes. That said, I would hope the reviewers would be respectful when pointing out these kind of issues anyway. My experience was very positive, even after making a silly mistake.

The biggest thing for me in taking my first step was finding the right issue and knowing it was okay to work on it. Once a reviewer responded saying a patch was welcome, I gave it a go. I was pretty anxious about the CL comments I would get after submitting it, but I think that may have been more attributable to being relatively new to open source contributing in general.

@ghost
Copy link

ghost commented Oct 6, 2017

@davecheney

This is concerning. Can you ask them for examples where the response has been unwelcoming?

Reading the issue tracker, your comments often come up as unwelcoming (at least, that's how it feels to me). Take, e.g., this issue (not the Go issue tracker, but an illustrative example) (screenshot). The reporter's command of English may not be perfect, but he obviously didn't ask questions, and your reply seems to be a bit... unrelated, a bit off.

And please, please if possible don't quote the comment you're answering to. This is an issue tracker, not an e-mail thread. Thanks.

(just curious, did that "Thanks." at the end make the comment feel less harsh?)

@davecheney
Copy link
Contributor

davecheney commented Oct 6, 2017

@opennota thank you for letting me know. I'm sorry that you interpreted this text as unwelcoming, it is certainly not how I intended it.

That text is my saved reply for directing people too ask questions elsewhere, not on the Go issue tracker. I can see how it would come off as brisk, and I'll work on improving the wording in the future.

@agnivade
Copy link
Contributor

agnivade commented Oct 6, 2017

FWIW, Dave's response seems perfectly fine to me. It should be noted that the issue was not even raised in the pertinent repo.

where if a new contributor makes a mistake, it's probably due to not knowing the context or the build tools well enough.

I see no issue with this at all. Why should a reviewer "switch mode" if the CL is sent by a first time contributor ? If I am a new contributor (which I was, until a few months back), and I get slapped in the wrist for making a dumb mistake, I will be happy to learn from my mistake and not repeat it in the future. That's how people learn and get better. The people reviewing my code are so much more knowledgeable than me. If they point out my mistakes, I take it as an opportunity to further my grasp on Go.

Being a maintainer of any reasonably big open source project is not easy. There will be PRs sent without reading CONTRIBUTING.md guidelines, without going through the PR template itself. Some people will just blindly click the 'Send Pull Request' button without even reading what is in front of them. I speak from experience.

I guess the point is people get offended much too easily without trying to understand the reviewer's point of view. Like Dave said - a little bit of understanding and compassion from both ends is the key to this.

P.S. All of the above is totally my opinion. From my personal experience, the Go core members are awesome. And I am glad to be a part of this.

@alexbrainman
Copy link
Member

I'm sorry that you interpreted this text as unwelcoming, it is certainly not how I intended it.

I hope this won't stop you doing the gardening, @davecheney Someone has got to pull the weeds and water the plants. Thank you.

Alex

@rsc
Copy link
Contributor

rsc commented Oct 9, 2017

/cc @andybons

@andybons
Copy link
Member

andybons commented Oct 9, 2017

I was speaking with someone today who mentioned that they read some of the CL's and issue responses and felt intimidated. Specifically they were worried that they would submit a CL and there would be a mistake with it and they'd get unfriendly feedback.

Whether they're a brand new contributor or a seasoned committer, unfriendly feedback isn't acceptable in any context and we should strive to do better for all reviews, not just to those people who are new to the project. Being nice and respectful is a baseline requirement for everyone, so appropriately calling someone out if they're not meeting that bar is important.

We have the "Thanks for your first contribution" commit message, but maybe we could add something else to flag for reviewers that the contributor is new to the project? Maybe gopherbot could add a "New-Contributor: +1" label to CL's where the submitter has submitted less than five CL's. Or a gopherbot message that says e.g. "Note to reviewers: This CL was submitted by someone new to the Go project! They might not have all of the context that others have."

I agree an additional signal could potentially be helpful, but it feels like it's optimizing the wrong part of the pipeline. I'd rather have something like concrete examples of situations where we can point people to reviews and say "this is an example of what it's like to contribute your first patch" so their anxiety about it is lowered.

@s-mang
Copy link
Contributor

s-mang commented Oct 10, 2017

Just stumbled upon this issue.
If I can add my drive-by 2 cents as an underrep'd minority, I personally would feel incredibly vulnerable having a label on my CL classifying me as even more of as an outsider than I already am.
Esp one that feeds the stereotype I already fight every day (newbie).

I would approach it instead by letting folks voluntarily identify as a 1st time contrib. Perhaps in private. Perhaps by documenting that if folks are nervous, they can reach out to X, Y or Z friendly person.

@kevinburke
Copy link
Contributor Author

Alright then, closing.

@s-mang
Copy link
Contributor

s-mang commented Oct 11, 2017

Hey Kevin! I do still think this is an important issue that we should address.
Maybe rewrite it as a problem instead of a solution?

(reopening, I hope that's ok)

@s-mang s-mang reopened this Oct 11, 2017
@rsc
Copy link
Contributor

rsc commented Oct 11, 2017

We have the "Thanks for your first contribution" commit message, but maybe we could add something else to flag for reviewers that the contributor is new to the project? Maybe gopherbot could ... [say] "Note to reviewers: This CL was submitted by someone new to the Go project! They might not have all of the context that others have."

The "Thank you for your first contribution" message was only just added. I would suggest that for now we assume that will be signal enough to reviewers that they should be extra kind when reviewing the CL, instead of adding a second automatic message that will be posted right next to the first one. Provided @kevinburke agrees, I would suggest we close this for now, and reopen in a few months if it becomes clear that more is needed.

@rsc
Copy link
Contributor

rsc commented Oct 11, 2017

Re labels, I would strongly prefer not to see a new label, for two reasons:

  1. Labels in Gerrit are a bit more heavy weight than on GitHub, because they have associated values and typically have strong semantic meaning, so they are much more emphasized in the UI. More labels will not make the Gerrit experience better overall. (Every CL by old contributors will say "New-Contributor: 0".)

  2. The tiny amount of text on a label fails to capture nuance and is too easy to misinterpret. In contrast the automatic "Thank you" message, by virtue of having more space, does (at least in my opinion) a very good job at setting the right tone.

@s-mang
Copy link
Contributor

s-mang commented Oct 11, 2017

@rsc I think the issue is that someone (people) are nervous or frightened to submit a first CL, and so they don't.
I think that's an issue worth keeping alive and trying to track progress on.
I also don't think sending a message after submitting a first CL is something that addresses this.

@andybons
Copy link
Member

I think that's an issue worth keeping alive and trying to track progress on.
I also don't think sending a message after submitting a first CL is something that addresses this.

Would it be better to either rename this something like “make it less scary to contribute your first patch“ only with better wording or would that derail the issue? I agree sending a message after they’ve already tried to contribute isn’t the best way to address this, but in its current form that’s what this issue is proposing.

@s-mang
Copy link
Contributor

s-mang commented Oct 11, 2017

yea, see my message earlier.

Maybe rewrite it as a problem instead of a solution?

@s-mang
Copy link
Contributor

s-mang commented Oct 12, 2017

@kevinburke hey, do you want to massage the issue a bit to state a problem?

I think you've probably got the most state on this ATM, since you just gave your talk at GothamGo. So I think you're the right person to own this RN if you're up for it.
I'm here to support you though, ofc.
Thanks again for bringing this up.

@ghost
Copy link

ghost commented Oct 26, 2017

Something perhaps tangent to this thread. Historically under Anglo-American jurisprudence, CLA signers, were these Parties to an 'action at law', would have their case heard in a Common Law Court, whereas of a 'suit in equity' being filed, e.g., Gebhart v. Belton, being as it was, of a sociological subject matter, this would oblige Parties' attendance in a Chancery Court.

Gebhart v. Belton, 33 Del. Ch. 144, 87 A.2d 862 (Del. Ch. 1952), aff'd, 91 A.2d 137 (Del. 1952)

@bradfitz
Copy link
Contributor

bradfitz commented Nov 6, 2017

I'd like to keep the bot's existing "Welcome new contributor!" message on Gerrit (thanks, @kevinburke!), but then also add a new section of "CLs from new contributors" to the top of https://dev.golang.org/ for contributors who've sent fewer than N CLs (or have been contributing less than N days), etc.

@rsc
Copy link
Contributor

rsc commented Nov 6, 2017

"Do things to encourage people to send CLs" is a separate topic from the original post. If anyone has concrete suggestions there, it would be best to open a new Github issue.

I think @bradfitz's suggestion is a good idea.

@rsc rsc changed the title proposal: add "New Contributor" label to CL's x/build/devapp: flag new contributors in CL list Nov 13, 2017
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Nov 13, 2017
@rsc rsc modified the milestones: Proposal, Unreleased Nov 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Builders x/build issues (builders, bots, dashboards) Proposal Proposal-Accepted
Projects
None yet
Development

No branches or pull requests