Supporting New Contributors

[mi...@bocoup.com]

Hey folks,

As a relative newcomer to the Gaia project (but a long-time user of GitHub), I
have some thoughts on how to improve the experience for contributors who
discover Gaia through its project page at https://github.com/mozilla-b2g/gaia .

I've already worked with Julien Wajsberg to create a Contributing.md file in
the root of the project [1]. GitHub.com gives special treatment to this file
[2]: users will be prompted to read it when they open new pull requests. I've
left it intentionally terse to avoid documentation rot--the important thing is
that users learn about the "Gaia/Hacking" wiki page [3]. This will help them
learn to create bugs on BugZilla to accompany their patches.

I think there is more that can be done, but these steps definitely require more
discussion before following through:

## Disabling GitHub Issues

The "Issues" feature on GitHub.com is optional; each project may disable it
through the Admin interface. Since the preferred method of tracking bugs is
BugZilla, this project does not need the feature.

More than being unecessary, I think that the "Issues" feature is potentially
harmful for this project:

1. It is confusing. New developers could not be faulted for assuming that it is
the preferred method of filing bugs (while in reality, BugZilla is the
preferred tool for tracking this information).
2. Left unmaintained, it is discouraging. New developers may assume that a
project with many many open issues on GitHub (over 600 at the time of this
writing [4]) does not value the input of most contributors.

## Pull Request Maintenence

The presence of many long-standing pull requests suggest that this is the
preferred method for submitting and discussing patches. Again, this is not the
case, and persisting hundreds of irrelevant open pull requests [5] will also
tend to discourage potential contributors.

We should consider closing all obsolete pull requests and promoting stricter
practices around managing new requests.

What do you think?

Mike

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=863443
[2] https://github.com/blog/1184-contributing-guidelines
[3] https://wiki.mozilla.org/Gaia/Hacking
[3] https://github.com/mozilla-b2g/gaia/issues
[4] https://github.com/mozilla-b2g/gaia/pulls

[Dietrich Ayala]

> ## Disabling GitHub Issues

Yes please. Been meaning to look into this for a while.

I will wait 24hrs for global feedback loop, and then disable if it's
not still under debate.

[Fabrice Desre]

Can we also disable github reviews? It's so annoying to not have the
reviews with the bugzilla context (and I'm not even talking about the
zillion mails we get for each sub comment that is made).

Fabrice
--
Fabrice Desré
b2g team
Mozilla Corporation

[tofumatt]

Like, pull requests? Isn't that the entire POINT of GitHub? Inline reviews with your code, encourages contributions from people not familiar with Bugzilla's awful and confusing review system.

You can always disable or filter the GitHub emails if you don't like them. They did a pretty extensive notification overhaul recently.

- tofumatt | http://tofumatt.com

> _______________________________________________
> dev-gaia mailing list
> dev-...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-gaia

[Fabrice Desre]

On 05/22/2013 10:33 AM, tofumatt wrote:
> Like, pull requests? Isn't that the entire POINT of GitHub? Inline reviews with your code, encourages contributions from people not familiar with Bugzilla's awful and confusing review system.

Pull requests are ok, even if we don't have tons of external
contributions anyway. But in any case, we now depend on a bugzilla bug
and not only a PR, so contributors will have to use bugzilla (having
persona login here is great to lower the entry barrier imho).

I'm also not convinced that github review system is much better than
splinter. They are different, you like one, I like the other.

> You can always disable or filter the GitHub emails if you don't like them. They did a pretty extensive notification overhaul recently.

But if I disable these emails, I have no way to know when a PR/bug is
reviewed. I could not find an option to receive only digests. Github
just doesn't fit big & very active projects like gaia imho.

[Julien Wajsberg]

Le 22/05/2013 19:09, Fabrice Desre a écrit :
> On 05/22/2013 09:56 AM, Dietrich Ayala wrote:
>>> ## Disabling GitHub Issues
>> Yes please. Been meaning to look into this for a while.
>>
>> I will wait 24hrs for global feedback loop, and then disable if it's
>> not still under debate.
> Can we also disable github reviews? It's so annoying to not have the
> reviews with the bugzilla context (and I'm not even talking about the
> zillion mails we get for each sub comment that is made).
>

We won't, at least not now...

This is not any different than reviews done on IRC, by the way.

[Fabrice Desre]

On 05/22/2013 11:27 AM, Julien Wajsberg wrote:
>
> We won't, at least not now...

Why?

> This is not any different than reviews done on IRC, by the way.

I really don't know what you're talking about.

[Andrew Sutherland]

On 05/22/2013 01:53 PM, Fabrice Desre wrote:
> But if I disable these emails, I have no way to know when a PR/bug is
> reviewed.

When people finish their reviews, they should either clear the "review?"
flag or set it to "review-" on the bug. I've started using "review-"
instead of clearing the review flag in spite of the negative connotation
to make it more clear to triagers/managers that the most recent state of
the pull request needs work. The flag then gets set to "review?" again
once a new update has been pushed.

Drive-by reviews or comments on the pull request will need to make some
comment on the bug (maybe set the feedback flag?).

Having said that, if you unsubscribe from watching the components in
question, you should still get e-mails from github when activity happens
on a pull request you've created, been name-checked on (ex:
@asutherland), or commented on.

Andrew

[Corey Frang]

On Tuesday, May 21, 2013 5:38:55 PM UTC-5, mi...@bocoup.com wrote:
> ## Disabling GitHub Issues

We should turn this off for sure.

> ## Pull Request Maintenence
>
> The presence of many long-standing pull requests suggest that this is the
> preferred method for submitting and discussing patches. Again, this is not the
> case, and persisting hundreds of irrelevant open pull requests [5] will also
> tend to discourage potential contributors.
>
> We should consider closing all obsolete pull requests and promoting stricter
> practices around managing new requests.
>
> What do you think?

I think if we land commits with "Closes gh-9999" in the commit message, this solves part of it.

We should make sure to close pull requests which will not land and/or work with the contributor to follow the proper contribution guidelines needed to help it land.

An open pull request should equate to an open work in progress on a bugzilla bug, if the bug closes, or the work is abandoned, the pull should be closed and the attachment either r+'ed or obsoleted.

Just my two cents.

[Gabriele Svelto]

I've tried to follow but I'm a bit confused at this stage... Until now
when I made a change to one of our GitHub repos I usually created a
pull-request, attached it to the bug and set the review? flag there.
Reviews would then arrive either on Bugzilla or on GitHub but more often
than not on GitHub. I usually responded by refreshing the pull-request
and asking for review again. Ultimately I would merge the final pull
request adding r=<reviewer> in the comment and pasted the link to the
merged version on GitHub.

Are we now trying to change this into adding a patch to the bug and
requesting the review there as per our Mercurial repos and iterating as
usual (attach, review, refresh, start over) and finally creating a
pull-request for the r+'d patch?

Gabriele

[Julien Wajsberg]

These days, I actually do both:
* a patch generated with "git format-patch -U8", that I attach on the
bug. I put the head of the patch (ie: the commit log and the statdiff)
as the comment text.
* a PR, so that Travis can run, and so that people that like to review
on Github can do that. I put the PR URL in the comment text too.

--
Julien

[Julien Wajsberg]

Le 22/05/2013 20:31, Fabrice Desre a écrit :
> On 05/22/2013 11:27 AM, Julien Wajsberg wrote:
>> We won't, at least not now...
> Why?

In the past months/years, we have empirically grown up a process that
works quite well and efficiently, and if you change this now, it will
take some time before the team is efficient again.

I'm not saying we should not try to target this in the long term though.

>
>> This is not any different than reviews done on IRC, by the way.
> I really don't know what you're talking about.

I mean you don't have the review history either on Bugzilla when you do
informal reviews on IRC instead of on Bugzilla.

--
Julien

[Dietrich Ayala]

I'm turning off "issues" today, as no one voiced any disagreement to that.

Regarding pull-requests: I think that pushing all patches back to
Bugzilla would be a huge step backward. I've been a proponent of
radically increasing/improving contribution vectors to Mozilla for a
long time, and IMO accepting pull-requests through Github is the
biggest win there.

I agree with Mike that stale requests is a problem. At that point you
*have* contributors, but you burn them, and they go away. Huge waste
of opportunity. I think the focus should be on how to handle that
problem, not reducing the opportunities to contribute.

[Fabrice Desre]

On 05/23/2013 01:01 AM, Gabriele Svelto wrote:

>
> Are we now trying to change this into adding a patch to the bug and
> requesting the review there as per our Mercurial repos and iterating as
> usual (attach, review, refresh, start over) and finally creating a
> pull-request for the r+'d patch?

There's no need for a pull-request if you do that. You can just push
your commits to gaia master/v1-train/...

[Fabrice Desre]

On 05/23/2013 03:05 AM, Julien Wajsberg wrote:
> Le 22/05/2013 20:31, Fabrice Desre a écrit :
>> On 05/22/2013 11:27 AM, Julien Wajsberg wrote:
>>> We won't, at least not now...
>> Why?
>
> In the past months/years, we have empirically grown up a process that
> works quite well and efficiently, and if you change this now, it will
> take some time before the team is efficient again.

I also heard that when we moved gaia from issues to bugzilla fwiw.

>>> This is not any different than reviews done on IRC, by the way.
>> I really don't know what you're talking about.
>
> I mean you don't have the review history either on Bugzilla when you do
> informal reviews on IRC instead of on Bugzilla.

That's very very unusual to do informal reviews on IRC. That's really
not a fair comparison with the lack of github comments history in
bugzilla given the amount of review happening on github.

[Fabrice Desre]

On 05/23/2013 08:42 AM, Dietrich Ayala wrote:
> I'm turning off "issues" today, as no one voiced any disagreement to that.
>
> Regarding pull-requests: I think that pushing all patches back to
> Bugzilla would be a huge step backward. I've been a proponent of
> radically increasing/improving contribution vectors to Mozilla for a
> long time, and IMO accepting pull-requests through Github is the
> biggest win there.

I agree that PR can lower the barrier for new contributors. But for
regular commiters I'm far from convinced that this benefits the project.
Also, do we have data on the number of PR from contributors were sent us
"out of nowhere"?

[Gabriele Svelto]

> There's no need for a pull-request if you do that. You can just push
> your commits to gaia master/v1-train/...

That sounds like a workflow I'd very much prefer as it's closer to our
bugzilla workflow. The only problem I see with this is that I've got
level-1 commit access on our Hg repositories so I would expect the same
on GitHub however I can merge my own pull requests already there so I
assume that's equivalent to level-3 which is weird since nobody vouched
for me.

Now in the past I've worked on large projects were all contributors had
commit rights but people would be allowed to push their own patches only
after review (GCC to be specific were they call that
write-after-approval status). That seemed to work well for them but it
does require a bit of discipline especially when educating new contributors.

Gabriele

[Dietrich Ayala]

No, but I don't expect it to be a silver bullet. As we've learned
elsewhere, varied contribution pathways are part of a whole
environment which can improve or impede contributions.

The fact that rotting pull-requests is a problem at all is interesting though :)

Also, "out of nowhere" is but *one* type of contributor. We also have:

* Volunteer contributors from many other sources
* Partner contributors
* Academic programs (capstone, thesis projects, etc)
* Contractor groups
* New employees

[Dietrich Ayala]

Fabrice, also I encourage you to participate in the Coding Stewards.
It's a group of Mozilla engineers who've been spending a little time
each week for a number of years talking about exactly these types of
things.

https://wiki.mozilla.org/Stewards/Coding

[Julien Wajsberg]

Le 23/05/2013 18:28, Fabrice Desre a écrit :
> On 05/23/2013 01:01 AM, Gabriele Svelto wrote:
>
>> Are we now trying to change this into adding a patch to the bug and
>> requesting the review there as per our Mercurial repos and iterating as
>> usual (attach, review, refresh, start over) and finally creating a
>> pull-request for the r+'d patch?

> There's no need for a pull-request if you do that. You can just push
> your commits to gaia master/v1-train/...
>

A pull request lets you run travis.

Of course we can still push manually if we want.

[mi...@bocoup.com]

Totally: there are contributors and reviewers who prefer GitHub for reviews and/or for workflow.

We could address this programatically--write a script that uses the GitHub API to close "stale" (however we want to define that term) pull requests. They could be closed with a message encouraging the author to re-open if still relevant, probably once again linking back to BugZilla and/or the "Gaia/Hacking" wiki.

[Dietrich Ayala]

Nobody suggested otherwise, so I've disabled Issues on Gaia project on Github. I'll send a top-level note to dev-gaia to make it extra clear.