<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Sat, May 27, 2017 at 12:53 AM, James E. Blair <span dir="ltr"><<a href="mailto:corvus@inaugust.com" target="_blank">corvus@inaugust.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">Joshua Hesketh <<a href="mailto:joshua.hesketh@gmail.com">joshua.hesketh@gmail.com</a>> writes:<br>
<br>
> On Fri, May 26, 2017 at 1:09 AM, James E. Blair <<a href="mailto:corvus@inaugust.com">corvus@inaugust.com</a>> wrote:<br>
</span><span class="">>> So I think that we should start out by simply silently ignoring any<br>
>> patchset elements in the URL.  We could consider having Zuul leave a<br>
>> note indicating that the patchset component is not necessary and is<br>
>> being ignored.<br>
>><br>
><br>
> Hmm, I'm not sure if that's the best way to handle it. If somebody clicks<br>
> the link they'll be shown a particular patchset (whether they are aware of<br>
> not) and it may cause confusion if something else is applied in testing.<br>
> zuul leaving a message could help clarify this, but perhaps we should<br>
> honour the patchset in the URL to allow for some very specific testing or<br>
> re-runs. This also links into what I was saying (in my now forwarded<br>
> message) about "tested-with" vs "must be merged first". We could test with<br>
> a patchset but that is irrelevant once something has merged (unless we add<br>
> complexity such as detecting if the provided patchset version has merged or<br>
> if it was a different one and therefore the dependency isn't met and needs<br>
> updating).<br>
><br>
> Either way I like the idea of zuul (or something) leave a message to be<br>
> explicit.<br>
<br>
</span>I think when someone uses Depends-On, they invariably mean "this change<br>
depends on this other change" not "this other patchset".  Referring to a<br>
previous patchset may have some utility, however, it would be<br>
counter-intuitive and doesn't help developers in the way that Zuul is<br>
designed to.<br>
<br>
Fundamentally Zuul is about making sure that it's okay to land a change.<br>
It creates a proposed future state of one or more repositories, and<br>
verifies that future state is correct.  Depending on a patchset would<br>
violate this in two ways.<br>
<br>
First, if A Depends-On: B and B is updated, there is no feedback to the<br>
developers whether the new revision of B is correct.  We use Depends-On<br>
not only to ensure that change A has a way to pass tests, but also that<br>
B is a correct change that enables the behavior desired in A.<br>
<br>
In other words, we're not answering the question "Is it okay to merge B?"<br>
<br>
Second, we would be creating a proposed future state that we know can<br>
not exist.  If A Depends-On: an old patchset of B, and we run that test,<br>
it would be pointless because we know that old patchset of B is not<br>
going to merge.  So we're not answering the question "Will A be able to<br>
merge?"<br>
<br>
We merge every change with its target branch before testing in check<br>
(rather than testing the change as it was written) for the same<br>
reason -- we test what *will be*, not *what was*.<br>
<br>
-Jim<br>
</blockquote></div><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">Sure, completely agree with everything you're saying. Mostly just thinking out loud.</div><div class="gmail_extra"><br></div><div class="gmail_extra">I still think we should try and be explicit about what zuul is doing. Leaving a comment when using a different patchset to what was in the commit message is one way of doing that.</div><div class="gmail_extra"><br></div><div class="gmail_extra">We could extend the 'start message' of zuul to explain what it is about to do. eg: "Testing change XYZ against $branch, with change XYZ applied to $project $branch, with..." etc. This could alternatively go into a "state" file (of sorts) in the zuul logs/output. ie, summarise what the zuul-cloner did into simple terms that doesn't involve large logs.</div><div class="gmail_extra"><br></div><div class="gmail_extra">Cheers,<br>Josh</div><div class="gmail_extra"><br></div></div>