<div dir="ltr">As I was building up the GitHub driver, I noticed that in both Gerrit and Github drivers we have some pipeline and trigger requirements that are specific to a proposed change: A Gerrit Change or a Github Pull Request. These types of requirements include open, current-patchset, and approvals. In GitHub land, this includes reviews, and could (in the future) include labels too. There is also commit status in Github land, which is actually attached to the commit itself, rather than the pull request, so commit status _could_ be used with push events<div><br></div><div>That said, there are event types in both gerrit and GitHub that occur outside of a Change. In Gerrit, ref-updated events happen when code is pushed or deleted (or merged?) to a branch (or a tag is added/removed?). Github has a similar event type, a "push" event.</div><div><br></div><div>The trouble occurs when a pipeline is configured to have a requirement that is "change type" specific, but the trigger is a push type event. When Zuul checks to see if the requirements are met there are tracebacks (at least in the case of current-patchset, I haven't tested the others yet). This is happening because the push type events create a Ref object, while the change type events create a Change object, and they have different base attributes. A Ref object does not include the attributes necessary to compute "current-patchset" data.</div><div><br></div><div>Now, I can easily patch around this to avoid the tracebacks, but it would require defining some behavior, and that should get some thought by a wider group of folks than me, myself, and I.</div><div><br></div><div>open: This could either always be true, or always be false.</div><div><br></div><div>current-patchset: This could always be true for push type events, or we could examine the target ref and make judgment as to if the hash in the event is still the "current" hash of the target.</div><div><br></div><div>approvals / reviews: Since there is no change associated with the push, there can't really be any approvals or reviews. Requiring an approval and getting a push event is... weird. Should we just pass push events through, ignoring required approvals? Or always block them, since there can't ever be an approval?</div><div><br></div><div>Github labels: This is like approvals/reviews. There is no pull request, there cannot be a label. We either need to always pass or fail this requirement.</div><div><br></div><div>Github status: Currently we only look at status for Change based events, but this could be extended to look at them on Ref based events too, but in a reasonably responsive Zuul set up the status on a commit may not be set by the time Zuul checks it for scheduling purposes (then again, the pipeline could be triggered BY the status being set, which brings up another hairball, where we're only mapping status events back to an open pull requests, we'd have to change all that around).</div><div><br></div><div>Personally, my opinions are that to avoid confusion, change type requirements should always fail on push type events. This means open, current-patchset, approvals, reviews, labels, and maybe status requirements would all fail to match a pipeline for a push type event. It's the least ambiguous, and promotes the practice of creating a separate pipeline for push like events from change like events. I welcome other opinions!</div><div><div><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><br></div><div dir="ltr">- jlk</div></div></div></div></div>
</div></div>