[OE-core] Patch merge process

[Martin Jansa]

On Thu, Sep 10, 2015 at 06:23:07PM +0300, Alexander Kanevskiy wrote:
> On Thu, Sep 10, 2015 at 5:18 PM, Martin Jansa <martin.jansa at gmail.com>
> wrote:
> 
> > On Thu, Sep 10, 2015 at 05:03:25PM +0300, Alexander Kanevskiy wrote:
> > > On Wed, Sep 9, 2015 at 12:06 AM, Martin Jansa <martin.jansa at gmail.com>
> > > wrote:
> > > Gerrit, despite its good ability to integrate into existing
> > infrastructures
> > > might be not the best solution for OE, and I must admit, gerrit is not
> > the
> > > best tool in the world for reviews, as there are limits in it like
> > missing
> >
> > I disagree, we can agree on that :).
> >
> >
> ok :)
> 
> 
> > > ability to review properly series of patches.
> >
> > It allows to review series of patches very well, newer versions also
> > show chains of reviews in better UI than older versions did.
> >
> > Worse problem is that such series need to be uploaded as series not as
> > individual patches, because gerrit cannot automagically discover such
> > dependencies between patches and cherry-pick them to single branch and
> > resolve conflicts if there are, but if you push whole series of patches
> > it will show it as chain and all works correctly.
> >
> >
> might be. I haven't checked latest versions 2.10+, but versions below were
> not up to the shape compared to e.g. pull-request workflow in GitHub.

GitHub's pull-request workflow model is even worse than e-mail reviews
on ML.

It doesn't even support doing inline comments, because as soon as the
"source" branch is rebased or just amended to address the review
comments from v1, it will loose reference to all these comments, because
the commits will look "new" (different git SHA).

That's completely useless for good review, I want to compare different
revisions of the same patch, to see what comments I had before and how
they were addressed in new version. Github doesn't support anything like
that, people are using different work-arounds like opening new
pull-request for each revision (which means you need to always push to
new branch) or listing the commit SHAs inside the global "comments"
section, so that very patient reviewer can open the older revisions and
do manual diff (by switching between 2 browser tabs back and forth
trying to spot the differences or whatever).

Atlassian Stash has exactly the same problem, ReviewBoard the same, they
all think that to address review comments you just add another commit on
top of your branch to fix the issues you introduced in submitted commits
- but that's useless for code quality, bisect-ability etc. We don't want
10 garbage changes with incorrect commit messages and 11th change on top
which fixes the issues, we're doing review so that each change is
meaningful, build-able, test-able and properly documented in commit
message - that's all I want to ensure by using code review tool and only
Gerrit allows to do it efficiently.

Gerrit shows all revisions of the same patch (as long as Change-Id is
the same) in the same UI, allows you to see diff between any 2
revisions including comments on them etc).

Only people who never got used to do detailed reviews in Gerrit can say
that Github's pull-request are "review-tool".

You can see similar discussion e.g. lately in systemd ML after they
moved to github "for better reviews".

> > > However, for people who don't like Gerrit, there is also always a
> > > possibility to use GitHub or something similar, like locally hosted
> > GitLab
> > > if OE don't want to be dependant on external service.
> >
> > I want to use it to share status of the meta-oe patches with contributors,
> > running my own GitLab locally won't help with this.
> >
> >
> GitHub's model is to fork repository, do any amount of modification in your
> own copy and then open "pull-request" for review where you just specify
> source/destination repositories and branches with set of changes.
> 
> Gitlab comment was practically to have similar to GitHub workflow, but
> instead of public's GitHub service to have instance on openembedded.org
> infrastructure, and thus requiring users to register accounts there.

But we want to depend on well-maintained external service like we depend
on github.com to mirror our repositories and we ask our users to use
github mirrors instead of git.openembedded.org.

Similarly we can depend on http://gerrithub.io/ to host our gerrit
instance to save some maintenance for people with access to
openembedded.org infrastructure.

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.openembedded.org/pipermail/openembedded-core/attachments/20150910/30f55701/attachment-0002.sig>