Re: Proposed Gerrit workflow (was: Re: Notes from Sharkfest '13)

[Michael Tuexen <Michael.Tuexen () lurchi franken de>]

On Jun 24, 2013, at 4:52 PM, Anders Broman <anders.broman () ericsson com> wrote:

> -----Original Message-----
> From: wireshark-dev-bounces () wireshark org [mailto:wireshark-dev-bounces () wireshark org] On Behalf Of Bálint 
> Réczey
> Sent: den 24 juni 2013 14:38
> To: Michael Tuexen
> Cc: Developer support list for Wireshark
> Subject: Re: [Wireshark-dev] Proposed Gerrit workflow (was: Re: Notes from Sharkfest '13)
>
> Hi Michael,
>
> 2013/6/24 Michael Tuexen <Michael.Tuexen () lurchi franken de>:
> > On Jun 23, 2013, at 8:58 PM, Bálint Réczey <balint () balintreczey hu> wrote:
> >
> > > Hi,
> > >
> > > 2013/6/22 Marc Petit-Huguenin <marc () petit-huguenin org>:
> > > > -----BEGIN PGP SIGNED MESSAGE-----
> > > > Hash: SHA256
> > > >
> > > > On 06/22/2013 09:43 AM, Bálint Réczey wrote:
> > > > > Hi Marc,
> > > > >
> > > > > 2013/6/22 Marc Petit-Huguenin <marc () petit-huguenin org>:
> > > > > > -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
> > > > > >
> > > > > > On 06/22/2013 03:47 AM, Bálint Réczey wrote:
> > > > > > > Hi All,
> > > > > > >
> > > > > > > 2013/6/21 Marc Petit-Huguenin <marc () petit-huguenin org>:
> > > > > > > > -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
> > > > > > > >
> > > > > > > > On 06/20/2013 04:52 PM, Guy Harris wrote:
> > > > > > > > > On Jun 20, 2013, at 2:58 PM, Marc Petit-Huguenin 
> > > > > > > > > <marc () petit-huguenin org> wrote:
> > > > > > > > >
> > > > > > > > > > On 06/20/2013 02:17 PM, Gerald Combs wrote:
> > > > > > > > > >
> > > > > > > > > > > Advantates: - I'm not sure that an in-house equivalent (e.g.
> > > > > > > > > > > Gerrit plus a private repository) would be better than what 
> > > > > > > > > > > Github offers.
> > > > > > > > > >
> > > > > > > > > > Yes, Gerrit is better than github:
> > > > > > > > >
> > > > > > > > > Presumably you mean "Gerrit plus a private repository is better 
> > > > > > > > > than github", as Gerrit, as far as I can tell, is just software 
> > > > > > > > > that works with a Git repository.
> > > > > > > >
> > > > > > > > Yes, although managing repositories being what Gerrit do, Gerrit 
> > > > > > > > without a least one repository would be a very boring application.
> > > > > > > :-)
> > > > > > >
> > > > > > > I have started describing a Gerrit based workflow which IMO would 
> > > > > > > fit to the project at http://wiki.wireshark.org/Development/Workflow .
> > > > > > > Please check it and share your opinion.
> > > > > >
> > > > > > "Code is building and tests are passing on all platforms. (Tests 
> > > > > > automatically start when at least one Core Developer gives +1 or 
> > > > > > +2 to prevent overloading or cracking the build servers.)"
> > > > > >
> > > > > > Why do not build and test all patchsets submitted?  Is that a 
> > > > > > limitation of the build servers?  Having Jenkins automatically 
> > > > > > verify your patchset is IMO one of the nice feature of Gerrit, and 
> > > > > > it will lower the workload of core devs if building and testing 
> > > > > > are done before they start looking at the patchset.
> > > > > Build can be triggered by patchset submissin, too, but it would 
> > > > > require more build server resources. Usually not the first version 
> > > > > of the changeset will be accepted especially from new contributors 
> > > > > and this means more builds.
> > > >
> > > > My view is the opposite: New contributors patchsets will probably be 
> > > > rejected anyway (does not build, does not pass test, etc...), so 
> > > > having the system doing that lowers the burden on core developers, 
> > > > who they can focus on more high level problems.
> > > >
> > > > > Note that Core Developers would not have to wait since they can 
> > > > > give +1 for their own changesets.
> > > > >
> > > > > The other reason behind requiring a +1 from someone we trust is 
> > > > > that otherwise it would be easy to prepare a changeset which does 
> > > > > unspeakable things to the build servers which we don't want to 
> > > > > happen. Without requiring +1 we would have to prepare build systems 
> > > > > to cope with malicious commits.
> > > >
> > > > That is a good point (basically because of the halting problem).  
> > > > But builds are done in isolation (i.e a git clone is done each 
> > > > time), so apart using too much resources or never ending, there is 
> > > > no harm that can be done to the infrastructure.  And there is a Jenkins plugin to abort a build if it is stuck.
> > > I was concerned about using the buildbots for attacking other 
> > > systems, too, but all of those threats can be handled and we have 
> > > time for setting up build bots properly.
> > >
> > > I have updated the proposal to start tests for every change and allow 
> > > Code Devs to bypass peer review.
> > >
> > > Comments are still welcome. :-)
> > Dear all,
> >
> > hmm. From a process point of view: How long do I need to wait until 
> > condition 2 is fulfilled. For me this looks like a race condition. If 
> > I get a 2+ before a 2- it gets in, if it is the other way, it doesn't...
> Devs coming late can discuss the change they don't agree with, like now. ;-)
>
> > Anyway... I'm not sure if this process really improves wireshark as a project.
> > It all depends on others willing to to do peer reviews. In an 
> > industrial environment, you can force developers to do that. In an open source community, you can't.
> > I'm not saying, that the peer reviews won't be done, but I'm not sure if they will.
> We already have code review in our process but it is performed in bugzilla or on the mailing lists. Gerrit will give 
> a better interface for those.
> We already checked interesting changes after commits, and Gerrit will help that, too.
>
> I've just checked LibreOffice's and Android's Gerrit installation and they seem to be making progress on reviews and 
> https://www.google.com/search?q=open+source+projects+per+review
> also lists several articles about peer-reviews performed in Open Source projects which make me think that we could do 
> it, too.
I just wanted to know if someone on this list is also involved
in an Open Source project doing reviews. That way we could benefit
from first class experience.
> > The current process puts responsibility on the core developer who 
> > commits a change. Personally, I don't think it is bad if this breaks 
> > the build on some buildbot, I only this it is bad if the committer 
> > doesn't care. This worked out pretty well in the past, I think. I only 
> > run the head version of wireshark and can usually build it (thanks to the Mac OS X buildbots).
> > If a core developer didn't want to take responsibility for a patch, he 
> > could contact others to get feedback on questions. This also worked in 
> > the past since you contacted people who also are interested in the subject.
> I'm generally satisfied with the quality of trunk and I'm proud to be part of the project.
> I also think if we could break trunk even less often, it would be even better and Gerrit would help that and would 
> also help discussing the patches.
>
> > The same responsibility applies for changes being compiled by the 
> > buildbots. Each such change comes from a core developer. I'm 
> > hesitating to allow an arbitrary patch to compile on the buildbots 
> > where we have no one being responsible for it in any way. Some of the 
> > buildbots run older software, some of them are not hardened in any way.
> If all the buildbots are running a newly cloned VM and limits network usage of the VM, I think we can be safe.
If we can change the buildbots running like that, you might be true. However,
the current way the Mac OS X buildbots are running is different. The versions
running currently on the buildbots are Leopard (PPC) and Snow Leopard (Intel)
and would only allow the server versions to be virtualised. That would mean
that we upgrade the buildbots to at least Lion...
> > Does someone have experience with an open source project comparable to 
> > Wireshark requesting peer review? Linux wants patches to be signed, 
> > but they have maintainers, Mozilla/Firefox has payed developers...
> https://www.google.com/search?q=open+source+projects+per+review lists a few in addition to the two I listed in the 
> proposal.
>
> > Don't get me wrong: I'm against introducing more procedural 
> > constraints, if it helps wireshark as a project. I just think that the 
> > current situation isn't too bad and I'm not sure how to make sure 
> > non-trivial peer reviews get done...
> I hope we could put together a pleasant wokflow which helps the project.
> I think we see Git bringing in a lot of improvements but with Git can't come without changing the workflow a bit. 
> Simply because Git is way faster than Subversion and it is too easy to push a lot of things to master accidentally.
> There needs to be someone (Linux's model) or something (Gerrit) preventing that.
>
> I think the proposed workflow with Gerrit would fit the project's way of doing development with minimal changes to 
> the current workflow.
>
> I also fear that per reviews (a good thing in theory) will slow the project down, I personally am not keen to move to 
> GIT at all and it will cause me a lot
> Of work on my internal setup and most probably reduce my involvement in the project over all. At the very least in 
> the sort time frame.
If consensus is that the review will be done, we can try that...

Regarding the buildbots, we need to see how to change things there... The
current way doesn't allow testing arbitrary patches...

Best regards
Michael
> Best regards
> Anders 
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe