[PATCH 04/10] models: Add 'status' model

Damien Lespiau damien.lespiau at intel.com
Thu Sep 24 20:38:50 AEST 2015


On Wed, Sep 23, 2015 at 10:14:05PM +0100, Finucane, Stephen wrote:
> > So it does mean my assumption was correct, eg. 'intel/license-check'
> > will be repeated for every patch. Hence abstracting a Test table we can
> > use for good.
> 
> Yes, but the context does not imply that the tests are the same.
> Please refer to the GitHub documentation for this. The non-uniqueness
> of this is important.
> 
> https://developer.github.com/v3/repos/statuses/

Oh, so you copied/pasted the github API. Sure that *can* work, but it
doesn't mean it is authority.

What does "the non-uniqueness of this is important" mean? I'm saying
that patch after patch, you'll still have
"continuous-integration/jenkins" repeated. I see some value in
extracting a Test object and having "statuses" refer to that test
object.

> 
> You are also assuming the "admins" for patchwork are the ones who
> would decide what tests are run. This is a dangerous assumption.

Why is that dangerous?

> Finally, how would you deal with the case where changes to different
> files would result in different tests?

Define the list of tests, skip them when not needed.

> > I don't think that a compelling argument. For instance, in that case, we
> > could have a more explicit "skip" or "non necessary" state for tests not
> > run and have more visibility on what the testing system actually does
> > while still tracking testing completion.
> 
> Again, patchwork should not be in charge of this. A developer or
> developer-provided script decides what tests to run. Patchwork just
> reports.

And I'm asking "why not?"

> I wasn't calling you stupid - I was trying to be friendly. Cop on.

That really doesn't go through.

> I'm following the proposal by Thomas Monjalon of 6WIND. This series
> works for that proposed model while yours does not. This has been
> agreed on the mailing list. If you have issues, I suggest taking it up
> in the original thread. Otherwise, can we please review this given in
> light of that proposal?
> 
> https://lists.ozlabs.org/pipermail/patchwork/2015-July/001363.html 

Your cover letter should have mentionned it, I'd have started by that.

-- 
Damien


More information about the Patchwork mailing list