[haiku-development] Re: cgit -> XX?
- From: Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>
- To: haiku-development@xxxxxxxxxxxxx
- Date: Fri, 15 Sep 2017 21:41:15 +0200
On Fri, Sep 15, 2017 at 12:29:36PM -0500, kallisti5 wrote:
After playing around with Gerrit... I have concerns that it is making
contributing to the project more difficult vs less.
(especially if we are going to require code review on *every* commit.
I'm strongly against that and am not giving my +1 to Gerrit due to it.)
It should not be mandatory for people with commit access. Use your
wisdom to decide when to ask for review.
I don't see why code reviews are a problem however, we already do them
after the fact on a mailing list. Doing them before pushing would avoid
reverting and amending changes in the main repo, thus keeping our git
history cleaner.
So we have two options:
- Make review mandatory, and we can stop doing after the fact reviews on
the haiku-commits mailing list.
- Allow people to push to the main repo directly, but then we need to
continue reviewing such changes using the mailing list.
I would prefer the first one, but I can understand people wanting to
bypass the review process in some cases. Beware if you break the build,
however. From my experience in other places (paid work), code review
does catch a lot of bugs, and overall, saves time. It also means the
reviewer gets more familiar with the code being reviewed, and as a
result, at the end you get 2 people who know the code (submitter +
reviewer), instead of just 1.
The learning curve to commit to Haiku is already "not github simple",
why go even further down that rabbit hole? We need more contributors,
not fewer.
This is approaching the problem at the wrong side.
Our most scarce resource is people with commit access. They are the one
we should focus on and make sure they can work with a smooth process.
Otherwise the solution is obvious: allow everything, github PRs,
submitting patches to Trac, setup Gerrit, and accept patches over IRC
and mailing lists and forums too, because all of this is "github easy".
But then it is a mess when it comes to tracking what is merged, what is
not, and getting feedback on the changes to the original patch
submitter.
If we had a situation where a lot of people had a fork of Haiku on
github, with lots of changes in there that they didn't bother to
upstream, then yes, you could argue that our upstreaming process is too
cumbersome and no one wants to use it. Looking at our forks at github,
this does not appear to be the case.
Moreover, I'm not sure making it easy to submit a patch would
necessarily attract contributors. We may get more "drive-by"
contributions: people scratch an itch, submit a patch, and never reply
to review comments or the like. So, we don't really get new contributors
this way. If people are stopped by just setting a git hook and pushing
to a gerrit instance, it's unlikely they would stay with the project for
very long anyway.
Given my experience trying to drive GCI students into submitting their
first patches, mentors usually spend considerable time trying to figure
out the state of their repos. Github is actually a rather complex
workflow: you have your local git copy, the upstream, and your fork. All
of these can get out of sync and you can easily mess up your commit
history.
Let's compare:
Github:
- Clone the sources
- Start hacking on them
- Decide to submit a PR
- Fork the repo
- Add the fork as upstream
- Push changes to fork (ideally in a new branch)
- Create PR
- Get a review
- Make changes
- git push -f to fork
- PR gets merged
Gerrit:
- Clone the sources and setup review hook
- Start hacking
- Decide to submit changes for review
- Use "git review" or manually add the change ID to the commits to push
- Push changes to gerrit
- Get a review
- Make changes, keeping the same ID in the commits
- git push
- changes get merged
I find the Gerrit flow a little simpler (no messing with forks). I agree
it may be a little less well-known, but that can be fixed with good
documentation on our side.
So, the more interesting question for me is: which tool is better for
existing contributors? Which can we enable them to review and merge patches
more efficiently? How can they simplify or streamline our existing
workflow (for example, removing the need for after-the-fact review and
embarassing reverts?)
Other related posts:
