On Mon, Jan 18, 2016 at 1:03 PM, Raghavendra Talur <rta...@redhat.com> wrote: > > > On Fri, Jan 15, 2016 at 4:22 PM, Niels de Vos <nde...@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.
I've made the Verified flag a `NoBlock` flag. No changes are merge-able only with (Code-Review+2 && Smoke+1 && CentOS-regression+1 && NetBSD-regression+1). > >> >> >> 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 <kshlms...@gmail.com> wrote: >> > > On Thu, Jan 14, 2016 at 5:12 PM, Niels de Vos <nde...@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 <nde...@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.mukherje...@gmail.com> >> > >>> >> wrote: >> > >>> >> >> > >>> >> > -Atin >> > >>> >> > Sent from one plus one >> > >>> >> > On Jan 12, 2016 7:41 PM, "Niels de Vos" <nde...@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 > > _______________________________________________ Gluster-infra mailing list Gluster-infra@gluster.org http://www.gluster.org/mailman/listinfo/gluster-infra