Open Stack

Joshua Hesketh <joshua.hesketh at gmail.com> writes:

> On Fri, May 26, 2017 at 1:09 AM, James E. Blair <corvus at inaugust.com> wrote:
>> So I think that we should start out by simply silently ignoring any
>> patchset elements in the URL.  We could consider having Zuul leave a
>> note indicating that the patchset component is not necessary and is
>> being ignored.
>>
>
> Hmm, I'm not sure if that's the best way to handle it. If somebody clicks
> the link they'll be shown a particular patchset (whether they are aware of
> not) and it may cause confusion if something else is applied in testing.
> zuul leaving a message could help clarify this, but perhaps we should
> honour the patchset in the URL to allow for some very specific testing or
> re-runs. This also links into what I was saying (in my now forwarded
> message) about "tested-with" vs "must be merged first". We could test with
> a patchset but that is irrelevant once something has merged (unless we add
> complexity such as detecting if the provided patchset version has merged or
> if it was a different one and therefore the dependency isn't met and needs
> updating).
>
> Either way I like the idea of zuul (or something) leave a message to be
> explicit.

I think when someone uses Depends-On, they invariably mean "this change
depends on this other change" not "this other patchset".  Referring to a
previous patchset may have some utility, however, it would be
counter-intuitive and doesn't help developers in the way that Zuul is
designed to.

Fundamentally Zuul is about making sure that it's okay to land a change.
It creates a proposed future state of one or more repositories, and
verifies that future state is correct.  Depending on a patchset would
violate this in two ways.

First, if A Depends-On: B and B is updated, there is no feedback to the
developers whether the new revision of B is correct.  We use Depends-On
not only to ensure that change A has a way to pass tests, but also that
B is a correct change that enables the behavior desired in A.

In other words, we're not answering the question "Is it okay to merge B?"

Second, we would be creating a proposed future state that we know can
not exist.  If A Depends-On: an old patchset of B, and we run that test,
it would be pointless because we know that old patchset of B is not
going to merge.  So we're not answering the question "Will A be able to
merge?"

We merge every change with its target branch before testing in check
(rather than testing the change as it was written) for the same
reason -- we test what *will be*, not *what was*.

-Jim