[Gluster-infra] Requesting separate labels in Gerrit for better testing results

Mon Jan 18 08:49:35 UTC 2016

On Mon, Jan 18, 2016 at 01:03:40PM +0530, Raghavendra Talur wrote:
> On Fri, Jan 15, 2016 at 4:22 PM, Niels de Vos <ndevos at redhat.com> wrote:
> 
> > On Thu, Jan 14, 2016 at 10:26:46PM +0530, Kaushal M wrote:
> > > I'd pushed the config to a new branch instead of updating the
> > > `refs/meta/config` branch. I've corrected this now.
> > >
> > > The 3 new labels are,
> > > - Smoke
> > > - CentOS-regression
> > > - NetBSD-regression
> > >
> > > The new labels are active now. Changes cannot be merged without all of
> > > them being +1. Only the bot accounts (Gluster Build System and NetBSD
> > > Build System) can set them.
> >
> 
> Thanks Kaushal !
> 
> 
> >
> > It seems that Verified is also a label that is required. Because this is
> > now the label for manual testing by reviewers/qa, I do not think it
> > should be a requirement anymore.
> >
> > Could the labels that are needed for merging be setup like this?
> >
> >   Code-Review=+2 && (Verified=+1 || (Smoke=+1 && CentOS-regression=+1 &&
> > NetBSD-regression=+1))
> >
> 
> I would prefer not having Verified=+1 here. A dev should not be allowed to
> override the restrictions.

That works for me too.

Niels

> 
> 
> >
> > I managed to get http://review.gluster.org/13208 merged now, please
> > check if the added tags in the commit message are ok, or need to get
> > modified.
> >
> > Thanks,
> > Niels
> >
> >
> > >
> > > On Thu, Jan 14, 2016 at 9:22 PM, Kaushal M <kshlmster at gmail.com> wrote:
> > > > On Thu, Jan 14, 2016 at 5:12 PM, Niels de Vos <ndevos at redhat.com>
> > wrote:
> > > >> On Thu, Jan 14, 2016 at 03:46:02PM +0530, Kaushal M wrote:
> > > >>> On Thu, Jan 14, 2016 at 2:43 PM, Niels de Vos <ndevos at redhat.com>
> > wrote:
> > > >>> > On Thu, Jan 14, 2016 at 11:51:15AM +0530, Raghavendra Talur wrote:
> > > >>> >> On Tue, Jan 12, 2016 at 7:59 PM, Atin Mukherjee <
> > atin.mukherjee83 at gmail.com>
> > > >>> >> wrote:
> > > >>> >>
> > > >>> >> > -Atin
> > > >>> >> > Sent from one plus one
> > > >>> >> > On Jan 12, 2016 7:41 PM, "Niels de Vos" <ndevos at redhat.com>
> > wrote:
> > > >>> >> > >
> > > >>> >> > > On Tue, Jan 12, 2016 at 07:21:37PM +0530, Raghavendra Talur
> > wrote:
> > > >>> >> > > > We have now changed the gerrit-jenkins workflow as follows:
> > > >>> >> > > >
> > > >>> >> > > > 1. Developer works on a new feature/bug fix and tests it
> > locally(run
> > > >>> >> > > > run-tests.sh completely).
> > > >>> >> > > > 2. Developer sends the patch to gerrit using rfc.sh.
> > > >>> >> > > >
> > > >>> >> > > > +++Note that no regression runs have started automatically
> > for this
> > > >>> >> > patch
> > > >>> >> > > > at this point.+++
> > > >>> >> > > >
> > > >>> >> > > > 3. Developer marks the patch as +1 verified on gerrit as a
> > promise of
> > > >>> >> > > > having tested the patch completely. For cases where patches
> > don't have
> > > >>> >> > a +1
> > > >>> >> > > > verified from the developer, maintainer has the following
> > options
> > > >>> >> > > > a. just do the code-review and award a +2 code review.
> > > >>> >> > > > b. pull the patch locally and test completely and award a
> > +1 verified.
> > > >>> >> > > > Both the above actions would result in triggering of
> > regression runs
> > > >>> >> > for
> > > >>> >> > > > the patch.
> > > >>> >> > >
> > > >>> >> > > Would it not help if anyone giving +1 code-review starts the
> > regression
> > > >>> >> > > tests too? When developers ask me to review, I prefer to see
> > reviews
> > > >>> >> > > done by others first, and any regression failures should have
> > been fixed
> > > >>> >> > > by the time I look at the change.
> > > >>> >> > When this idea was originated (long back) I was in favour of
> > having
> > > >>> >> > regression triggered on a +1, however verified flag set by the
> > developer
> > > >>> >> > would still trigger the regression. Being a maintainer I would
> > always
> > > >>> >> > prefer to look at a patch when its verified  flag is +1 which
> > means the
> > > >>> >> > regression result would also be available.
> > > >>> >> >
> > > >>> >>
> > > >>> >>
> > > >>> >> Niels requested in IRC that it is good have a mechanism of
> > getting all
> > > >>> >> patches that have already passed all regressions before starting
> > review.
> > > >>> >> Here is what I found
> > > >>> >> a. You can use the search string
> > > >>> >> status:open label:Verified+1,user=build AND
> > label:Verified+1,user=nb7build
> > > >>> >> b. You can bookmark this link and it will take you directly to
> > the page
> > > >>> >> with list of such patches.
> > > >>> >>
> > > >>> >>
> > http://review.gluster.org/#/q/status:open+label:Verified%252B1%252Cuser%253Dbuild+AND+label:Verified%252B1%252Cuser%253Dnb7build
> > > >>> >
> > > >>> > Hmm, copy/pasting this URL does not work for me, I get an error:
> > > >>> >
> > > >>> >     Code Review - Error
> > > >>> >     line 1:26 no viable alternative at character '%'
> > > >>> >     [Continue]
> > > >>> >
> > > >>> >
> > > >>> > Kaushal, could you add the following labels to gerrit, so that we
> > can
> > > >>> > update the Jenkins jobs and they can start setting their own
> > labels?
> > > >>> >
> > > >>> >
> > http://review.gluster.org/Documentation/config-labels.html#label_custom
> > > >>> >
> > > >>> > - Smoke: misc smoke testing, compile, bug check, posix, ..
> > > >>> > - NetBSD: NetBSD-7 regression
> > > >>> > - Linux: Linux regression on CentOS-6
> > > >>>
> > > >>> I added these labels to the gluster projects' project.config, but
> > they
> > > >>> don't seem to be showing up. I'll check once more when I get back
> > > >>> home.
> > > >>
> > > >> Might need a restart/reload of Gerrit? It seems required for the main
> > > >> gerrit.config file too:
> > > >>
> > > >>
> > http://review.gluster.org/Documentation/config-gerrit.html#_file_code_etc_gerrit_config_code
> > > >
> > > > I was using Chromium and did a restart. Both hadn't helped. I'll try
> > again.
> > > >>
> > > >> Niels
> >
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://www.gluster.org/pipermail/gluster-infra/attachments/20160118/6af3608e/attachment.sig>