[dmd-internals] GitHub commit status API

Brad Roberts braddr at puremagic.com
Thu Sep 6 10:07:48 PDT 2012


On 9/6/2012 2:55 AM, Don Clugston wrote:
> On 6 September 2012 09:31, Brad Roberts <braddr at puremagic.com> wrote:
>> On 9/6/2012 12:26 AM, Don Clugston wrote:
>>> On 4 September 2012 19:43, Brad Roberts <braddr at puremagic.com> wrote:
>>>> Hrm.. there's one important difference between the way this api is designed and the way the auto-tester tests the pull
>>>> requests.  The api tags a specific sha, which would be the tip of the pull requests revision history.  That's fine if
>>>> the merge to master is purely a fast-forward merge (ie, the pull request is based on the tip of the master tree and is
>>>> purely additive).  However, if the pull request is NOT based off the tip of the tree, then a merge is involved and the
>>>> auto-tester is testing the result of that merge, NOT the pull request pre-merge.
>>>>
>>>> I can go ahead and apply the status to the pull requests tip sha, but it'll be a little misleading in that it's entirely
>>>> possible that the pull request builds w/o a merge to master but doesn't with.  Likely to be a fairly rare occurrence,
>>>> but worth noting.
>>>
>>> It's not rare at all, I've seen it very many times.
>>
>> Sorry, I was excluding the case where it's not mergable.  There's a lot of cases where it builds w/in just the pull
>> request, but isn't mergeable due to conflicting changes.  I think that case is fine to report failure for, since it
>> requires attention before the pull request is useful.  Are you aware of a significant number that _do_ merge but then
>> fail to build/test post-merge but succeed w/o a merge?
> 
> I've definitely seen some, usually related to forward references (any
> compiler change that changes the order of evaluation can easily cause
> other stuff to break).
> 
> I presume this would at least this would still catch the case where it
> merges but isn't compatible with druntime or phobos? There are quite a
> few open pull requests right now where that's true (a compiler change
> that worked with an old Phobos, but not with the current one).

I don't intend to change how the tester works (at least not due to the statuses api) unless we want it to be changed.  I
think the current way is better than testing the pull as-is (ie, without merging).

What I'm looking at is the issues with using the statuses api (which I really like the idea of.. far better than using
greasemonkey to wedge in changes to the ui) and the ways that it has issues with how the tester currently operates.

Later,
Brad




More information about the dmd-internals mailing list