[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