gerrit for release branches (bots ftw?)

classic Classic list List threaded Threaded
8 messages Options
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

gerrit for release branches (bots ftw?)


Hi all,

reviewing patches for a release branch is currently a hassle:

submitter:
- find cgit link
- write to mailing list with cgit link asking for review

reviewer:
- check out the release branch
- cherrypick commit on release branch by exctracting the commit-id from the cgit link
- review the commit
- push commit on the release branch
- mail to list that the stuff is pushed

using gerrit its a bit simpler for the reviewer:
- check out the release branch
- gerrit-cherry-pick the commit
- push the commit to the release branch (gerrit will take care of notifying the submitter

but the submitter has a bit more to do:
- check out the release branch
- cherry-pick the commit
- push to refs/for/libreoffice-3-5

While that is rather simple, it could be scripted even simpler. I submitted a bug to gerrit wrt this:

 http://code.google.com/p/gerrit/issues/detail?id=1446

but maybe I am just overlooking a best practice there. If not, we might
alternatively have either a email or an IRC bot, which you send a message like:

 releasereview 213d5355d78a0a690e366645d6416f4a8fe5e666 libroffice-3-5

and it will do all of the above on its own. Opinions on that? Is it better to
have an IRC or an Email bot?

Best,

Bjoern
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Petr Mladek Petr Mladek
Reply | Threaded
Open this post in threaded view
|

Re: gerrit for release branches (bots ftw?)

Bjoern Michaelsen píše v Po 18. 06. 2012 v 15:27 +0200:
> Hi all,
>
> reviewing patches for a release branch is currently a hassle:

I see the main advantages of gerrit in the following three things:

        + be able to check build on more platforms before pushing;
          useful for more complex changes
        + buildability is already tested when I start reviewing patch
          => be more sure when approving trivial changes even for
                   branches where I do not have local build
        + have nice overview of not-yet handled patches:
                + be sure that all patches are committed before release
                + do not miss contributed patches

Though, there are other solutions for the above advantages:
        + tinderboxes could build branches
        + we have less build breakages last months
        + making sure that all patches are integrated affects primary
          only one person (me :-)
        + IMHO, we do not miss too many patches; also people could ask
          again; they need to be patient and a bit motivated to be able
          to work on LO anyway

We need to be sure that advantages are bigger than disadvantages.
We need to make sure that all processes are either easier or not too
much more complicated. I am not convinced yet, see below.


> submitter:
> - find cgit link
> - write to mailing list with cgit link asking for review

This handles only people with commit access and review for stable
branches. It does not handle newbies.


> reviewer:
> - check out the release branch
> - cherrypick commit on release branch by exctracting the commit-id from the cgit link
> - review the commit
> - push commit on the release branch
> - mail to list that the stuff is pushed
>
> using gerrit its a bit simpler for the reviewer:
> - check out the release branch
> - gerrit-cherry-pick the commit
> - push the commit to the release branch (gerrit will take care of notifying the submitter

Hmm, this is not much encouraging:

        + "git push" is easy.
        + if there are conflict, who and how would resolve them in
          gerrit?
        + you need to write a comment when approving the change anyway;
          some more words might motive the volunteer to come again, so
          the gerrit tool might be rather antisocial


> but the submitter has a bit more to do:
> - check out the release branch
> - cherry-pick the commit
> - push to refs/for/libreoffice-3-5

This is even worse. We rather need to lower the barriers and make things
easier.


> While that is rather simple, it could be scripted even simpler. I submitted a bug to gerrit wrt this:
>
>  http://code.google.com/p/gerrit/issues/detail?id=1446
>
> but maybe I am just overlooking a best practice there.

The command is too long. I am afraid that you need to come up with
something easier to get it adopted.


>  If not, we might
> alternatively have either a email or an IRC bot, which you send a message like:
>
>  releasereview 213d5355d78a0a690e366645d6416f4a8fe5e666 libroffice-3-5
>
> and it will do all of the above on its own. Opinions on that? Is it better to
> have an IRC or an Email bot?

This looks better. It similar to the message from the openSUSE Build
Service that I attached to the other mail.

I prefer mail. irc is too distracting because it forces you to stop an
action and solve the other problem immediately. I am getting very
nervous when the stack of interruptions is more than 4. Also people need
to sleep some time and irc is still rolling.

I still see it optimistic. I am just not happy with the style of pushing
this solution. You direct style brought strong offense from others. It
brought offense from you as well. For me, it is harder to be
constructive in this atmosphere. I would prefer if you motivate people
rather than scary them. Also, you should hear their needs and offer
solutions. If you are not sure about their needs, you should ask for
more details.

Thanks in advance for patience ;-)


Best Regards,
Petr

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Petr Mladek Petr Mladek
Reply | Threaded
Open this post in threaded view
|

Re: gerrit for release branches (bots ftw?)

In reply to this post by Bjoern Michaelsen
Bjoern Michaelsen píše v Po 18. 06. 2012 v 15:27 +0200:

> submitter:
> - find cgit link
> - write to mailing list with cgit link asking for review
>
> reviewer:
> - check out the release branch
> - cherrypick commit on release branch by exctracting the commit-id from the cgit link
> - review the commit
> - push commit on the release branch
> - mail to list that the stuff is pushed
>
> using gerrit its a bit simpler for the reviewer:
> - check out the release branch
> - gerrit-cherry-pick the commit
> - push the commit to the release branch (gerrit will take care of notifying the submitter
>
> but the submitter has a bit more to do:
> - check out the release branch
> - cherry-pick the commit
> - push to refs/for/libreoffice-3-5

One more idea. This is not easy to compare. I wonder if you could create
a table when you will compare the basic operations side by side. I mean
something like https://git.wiki.kernel.org/index.php/GitSvnCrashCourse


For example:

1. Commit local change to master:


current:                        gerrit:
git commit -a                   ???
git pull -r
git push



2. Approve patch from new contributor:

current:

# save patch to disk            gerrit:
git am p.patch                  ???
make
git pull -r
git push



3. Send patch for review into branch 3-5, 3-5-5

current:                                          gerrit:
# find URL in cgit or do git format-patch         ???
# write mail


It would be great if you could write this into wiki. IMHO, it would be
very useful documentation. I am sure that there will be more scenarios
that will need explanation, e.g.:

        + solving build failures
        + solving conflicts when pushing into real master
        + what to do if cherry-pick conflicts
        + building on more platforms (big win for gerrit)


Best Regards,
Petr

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

Re: gerrit for release branches (bots ftw?)

In reply to this post by Petr Mladek
Hi Petr,

On Tue, Jun 19, 2012 at 06:58:33PM +0200, Petr Mladek wrote:
> > submitter:
> > - find cgit link
> > - write to mailing list with cgit link asking for review
>
> This handles only people with commit access and review for stable
> branches. It does not handle newbies.

Right, see subject. To send a patch for review to master anyone can just push
to refs/for/master on the gerrit repo -- everyone means everyone with a OpenID
somewhere, no fd.o account needed. People with commit access on fd.o will
continue to have direct push access to master on fd.o, if they pinged Norbert
or me about it after the first login as asked to in the mail one month ago.

> > reviewer:
> > - check out the release branch
> > - cherrypick commit on release branch by exctracting the commit-id from the cgit link
> > - review the commit
> > - push commit on the release branch
> > - mail to list that the stuff is pushed
> >
> > using gerrit its a bit simpler for the reviewer:
> > - check out the release branch
> > - gerrit-cherry-pick the commit
> > - push the commit to the release branch (gerrit will take care of notifying the submitter
>
> Hmm, this is not much encouraging:

This is not how it would end up finally, but just the transitional until in
about one month. I expect everyone to finally have logged into gerrit by then
so we could give them them direct commit rights (bypassing review) on master,
whoever is _still_ missing by then might indeed run into a minor interruption
in direct commit access.

> + if there are conflict, who and how would resolve them in
>           gerrit?

I dont assume too many conflicts happening on patches for release branches as
those are already rebased to the release branch by the submitter (or his
friendly helper bot, see below). For the few remaining conflicts, gerrit will
just ask you to rebase/resolve and resubmit.
Note again, that everyone who has commit access on fd.o right now will have
direct commit acccess on gerrit, so as a fallback you will be able to just do
stuff as you did before on the gerrit repo.

Note also that most of the annoying complexity of the current setup is
transitional an will be gone once we have everything on the gerrit repo.
Everything your could do on the fd.o, you will be able to do on the gerrit repo
(if you got your setup in time). In ADDITION, you can do the code review stuff
there.

> + you need to write a comment when approving the change anyway;
>           some more words might motive the volunteer to come again, so
>           the gerrit tool might be rather antisocial

Once gerrit owns the repo, you just write a +1/+2 review with a nice "thanks"
note and gerrit pushes the patch itself. Push to fd.o is just a temporary
workaround as long as we have not given all (or at least most) people who had
direct commit access on fd.o the same on gerrit, so that we can switch over.

> This is even worse. We rather need to lower the barriers and make things
> easier.

Yes, that what the bot is for.

> >  If not, we might alternatively have either a email or an IRC bot, which
> >  you send a message like:
> >
> >  releasereview 213d5355d78a0a690e366645d6416f4a8fe5e666 libroffice-3-5
> >
> > and it will do all of the above on its own. Opinions on that? Is it better to
> > have an IRC or an Email bot?
>
> This looks better. It similar to the message from the openSUSE Build
> Service that I attached to the other mail.

Also note that this would cherry-pick _before_ the review - so if there are
conflict, it can complain directly and not leave the dirty work to the
reviewer.

> I still see it optimistic. I am just not happy with the style of pushing
> this solution. You direct style brought strong offense from others.

Unfortunately, I saw no other way to bring this forward. I saw the sentiment "I
have no idea what it does, will not give any feedback on what I need it to be
to be usable to me, but I will retain the right to complain viciously about it"
for far too long. I expect people -- without haste, but in due time -- to have
a look at gerrit and give qualified feedback on what exactly is wrong with it
and where we still need tweaking.

After some good initial flameage, today provided some good qualified feedback
in the end -- while before, mails to this topic fell on no response. To prevent
further flameage, it might be helpful for all to get involved with the gerrit
discussion _early_ on -- it prevents a lot of further bad blood.

Best,

Bjoern
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Norbert Thiebaud Norbert Thiebaud
Reply | Threaded
Open this post in threaded view
|

Re: gerrit for release branches (bots ftw?)

In reply to this post by Petr Mladek
On Tue, Jun 19, 2012 at 11:58 AM, Petr Mladek <[hidden email]> wrote:
>
> This handles only people with commit access and review for stable
> branches. It does not handle newbies.

commit upstream first... so commit to master
in most case adding a patch to a release branch consist in
cherry-picking it from master... so it is already committed somehow,
regardless of the status of the original author.

'newbies' today do not push anything at all to master or otherwise..
on the other hand, one could push a commit for consideration for a
release branch without having commit access at all: so gerrit would
actually be more newby friendly in that regard

>
>
> Hmm, this is not much encouraging:
>
>        + "git push" is easy.

When gerrit handle the main repo (i.e fdo becomes read-only), the
workflow would be simplified.

>        + if there are conflict, who and how would resolve them in
>          gerrit?

the conflict can show up at 2 stages:
1/ when the commit is cherry picked fo consideration for release
branch, beore it is pushed to gerrit. tha case is handle the same way
it is today
2/ when another commit hit the release branch before the current
commit is 'merged'. that case should be ery rare and when that appen
the 'Publish' operation should fail.
at that point one need to go back to one;s tree, pull -r fix the
problem and push again the newly updated patch for review.
gerrit use the ChangeId (which would remain teh same here) to identify
that you are pushing a new version of the 'same' patch. at that point
we are back in the normal workflow

>        + you need to write a comment when approving the change anyway;
>          some more words might motive the volunteer to come again, so
>          the gerrit tool might be rather antisocial

You can and are encouraged to write comment in gerrit when
Reviewing/Verifying... There i no difference there. you could Reply-to
with just 'Thanks, pushed' or a longer message, just like you can be
as verbose or terse in the message that accompany your +2 on Review.

Norbert
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Bjoern Michaelsen Bjoern Michaelsen
Reply | Threaded
Open this post in threaded view
|

Re: gerrit for release branches (bots ftw?)

In reply to this post by Petr Mladek
On Tue, Jun 19, 2012 at 07:57:30PM +0200, Petr Mladek wrote:
> One more idea. This is not easy to compare. I wonder if you could create
> a table when you will compare the basic operations side by side. I mean
> something like https://git.wiki.kernel.org/index.php/GitSvnCrashCourse

Started here:

 https://wiki.documentfoundation.org/Development/gerrit/compare

Best,

Bjoern
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Ivan Timofeev Ivan Timofeev
Reply | Threaded
Open this post in threaded view
|

Re: gerrit for release branches (bots ftw?)

In reply to this post by Bjoern Michaelsen
Hi Bjoern,

On 18.06.2012 17:27, Bjoern Michaelsen wrote:
> we might
> alternatively have either a email or an IRC bot, which you send a message like:
>
>   releasereview 213d5355d78a0a690e366645d6416f4a8fe5e666 libroffice-3-5
>
> and it will do all of the above on its own. Opinions on that? Is it better to
> have an IRC or an Email bot?

Email! I would appreciate if I don't get forced to learn/use IRC
(where:
"learn" == "read some manual, understand nothing, have a headache" and
"use" == "try to use, do mistakes, look like an oaf, have a headache
again" :)  Is that feasible?

Thanks,
  Ivan
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Petr Mladek Petr Mladek
Reply | Threaded
Open this post in threaded view
|

Re: gerrit for release branches (bots ftw?)

In reply to this post by Bjoern Michaelsen
Bjoern Michaelsen píše v Út 19. 06. 2012 v 21:27 +0200:
> On Tue, Jun 19, 2012 at 07:57:30PM +0200, Petr Mladek wrote:
> > One more idea. This is not easy to compare. I wonder if you could create
> > a table when you will compare the basic operations side by side. I mean
> > something like https://git.wiki.kernel.org/index.php/GitSvnCrashCourse
>
> Started here:
>
>  https://wiki.documentfoundation.org/Development/gerrit/compare

Thanks for this page. I think that it is pretty useful and promising.

I am still a bit scared when I imagine the many branches for each user
and cherry-picking between them. I know that most actions will be
handled by gerrit. Maybe, I just do not know some git tricks. Anyway, I
hope that I will not get lost ;-)

Thanks for answering all the questions. You know, it is a big switch and
people do not know what to expect. Most people do not have time to play
with gerrit to discover everything themselves.


Best Regards,
Petr

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice