Open Stack

On Sat, May 27, 2017 at 12:53 AM, James E. Blair <corvus at inaugust.com>
wrote:

> 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
>


Sure, completely agree with everything you're saying. Mostly just thinking
out loud.

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.

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.

Cheers,
Josh
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-infra/attachments/20170529/2e6b5522/attachment.html>