Gerrit issue with drafts

[Jens Löök]

Hi
I sent this issue on our internal mailing list at Ericsson and it was suggested to post it here as well it would be interesting to hear your thoughts on this matter. 

I did a quick search but nothing to obvious came up so I hope I'm not posting something that comes up a lot here. 

We have defined a work flow where we use drafts in Gerrit to lighten the load on our build machinery a bit. The scenario is that you can get the review out of the way before you actually trigger the build and test jobs.

We are using "git-review" addon to submit changes to Gerrit and we have changed the default behavior of git-review to always create drafts, you can still submit with -P to get a public change (it is configurable for each repo if the default action should be publish or draft).

Now to our issue, if you have a change that contains one or several draft patch-sets and then you make the change public either by pushing the Publish button in the gui or by creating a new patch-set that is public our review job will be triggered. And if you need to create additional patch-sets to fix any problem in the review job all new patch-sets will trigger the review job no matter if you send them to refs/drafts och refs/publish. If you create a new draft change-set on top of a public change-set the review job will be kicked off because the "patchset-created" message from Gerrit contains "status":"NEW" but in the GUI the new patch-set will have status DRAFT and you will need to do publish before you can do submit and this will trigger two review-jobs for the same patch-set.

To me it seems like Gerrit is a bit inconsistent here and the behavior should be changed so that
  - a new draft patch-set on top of a public patch-set does not trigger a build, the message from Gerrit should still contain"status":"DRAFT" .
  or
  - a new draft patch-set on top of a public patch-set becomes a public change by default and you do not need to publish it before it can be submitted.

[Luca Milanesio]

Seems like a Gerrit-trigger-plugin issue rather than Gerrit.

Have you shared it on the Jenkins mailing list ?

Sent from my iPhone

--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Jens Löök]

I don't think so, as far as I can tell it is not possible to figure out that a draft on top of a public change-set actually is a draft from the message that is sent. But in the Gerrit gui the new change-set is marked as a draft. So the message that is picked up by the Gerrit trigger in Jenkins does not get the same information as the gui.

/Jens 

[Alex Blewitt]

The gerrit trigger plugin only listens for patch set created items. This is not created for items that are in draft and then published though I believe there is a separate event for that. 

It would make sense IMHO if gerrit sent a patch set created event when a draft is published as the event should represent "what is created" and not "where it came from"

Alex 

Sent from my iPhone 5

[Marcelo Ávila]

Alex,

The problem does not happen when a draft is published. The problem happens when a draft is pushed to a (already) published change. Jens is saying that Gerrit is not acting coherently in this situation.

--

Marcelo Ávila de Oliveira

CPqD - Information Technology Engineer

Tel.: +55 19 3705-4125

mav...@cpqd.com.br

www.cpqd.com.br

[Luca Milanesio]

Really like the "Build +1" flag option: it would be an even more explicit tracking of "ready to be built" status, to avoid the painful and expensive waste of build resources.

Luca.

On 8 May 2014, at 09:19, Sandell, Robert <Robert....@sonymobile.com> wrote:

Perhaps Gerrit is sending patchset-created events to users that are invited to the draft change, and since one patch set is revealed to the Jenkins user and Jenkins then has posted a comment on the patch set it’s considered to be able to view the draft.

 

Unless you have other reasons for using the draft method than to hide the changes from Jenkins; you could change your jobs to trigger when the patch set gets a “code review +2” or any custom label like “build +1”. It would have the same effect in that it would not trigger until a code review is done.

 

 

Robert Sandell

Sony Mobile Communications

Tel: +46 10 80 12721

sonymobile.com

 

From: jenkin...@googlegroups.com [mailto:jenkin...@googlegroups.com] On Behalf Of Jens Löök
Sent: den 7 maj 2014 15:31
To: jenkin...@googlegroups.com
Cc: Jens Löök
Subject: Re: Gerrit issue with drafts

 

I don't think so, as far as I can tell it is not possible to figure out that a draft on top of a public change-set actually is a draft from the message that is sent. But in the Gerrit gui the new change-set is marked as a draft. So the message that is picked up by the Gerrit trigger in Jenkins does not get the same information as the gui.

 

/Jens 

Den onsdagen den 7:e maj 2014 kl. 14:01:30 UTC+2 skrev lucamilanesio:

-- 
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.

[Sven Selberg]

The 'NEW' status is the status of the Change, since the Change isn't a draft and hasn't yet been submitted or abandoned. 

If you check out the PatchSetAttribute part of the PatchSetCreatedEvent you will (hopefully) find the isDraft flag is set to 'true'.

I believe that firing a PatchSetCreated event when a draft is submitted is correct, the patch set has been created even though the status is 'draft', and anyone who can see the 'draft' should be notified.

Submitting a patch set does not a patch set create.

Like Bobby said this has no bearing on whether the build is triggered or not, but I had to defend Gerrit a bit here :-)
/Sven

[Jens Löök]

Yes I missed the isDraft flag previously. I still think it is a bit confusing that there is a global state for the change and that each patch-set can have another state. 

I did a quick check 

- Submitted a new draft

- Submitted a new public patch-set -> build triggered

- Removed the Jenkins build users from reviewers

- Submitted a new draft patch-set -> build triggered 

Everyone listening to the Gerrit events get all events I guess, it is just a matter of how to interpret them. So I guess it is the behavior of the Gerrit trigger in Jenkins that needs to be modified. Maybe an option to ignore isDraft:true for patchset-created messages. 

/Jens

[David Ostrovsky]

Am Freitag, 9. Mai 2014 09:21:52 UTC+2 schrieb Jens Löök:

Yes I missed the isDraft flag previously. I still think it is a bit confusing that there is a global state for the change and that each patch-set can have another state. 

Well, nobody ever told you that Gerrit DRAFT workflow is easy to grasp ;-)

So an open change can have two states: new & draft.

Patch set can be a regular patch set or a draft patch set.

Note: there are two independent attributes in the data model that control this behavior.

So it's perfectly OK to have:

change | patch set

------------------

new    | new

draft  | draft

new    | draft