Hi all,

I don't know if you've seen this, but I thought I'd let you know:

https://github.com/blog/1227-commit-status-api

It does a similar job to what the auto-tester GreaseMonkey scripts already do but it's built into GitHub and modifies the merge button to make it more obvious. Could be worth looking into adding to the auto-tester at some point.

I would assume at some point they'll build on this to allow pull requests to be triaged based on the information.

-- 
Robert

Looks simple enough.  I'll try to get it done tonight.

On 9/4/2012 10:23 AM, Robert Clipsham wrote:
> Hi all,
> 
> I don't know if you've seen this, but I thought I'd let you know:
> 
> https://github.com/blog/1227-commit-status-api
> 
> It does a similar job to what the auto-tester GreaseMonkey scripts already do but it's built into GitHub and modifies the merge button to make it more obvious. Could be worth looking into adding to the auto-tester at some point.
> 
> I would assume at some point they'll build on this to allow pull requests to be triaged based on the information.
> 
> -- 
> Robert

_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

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.

On 9/4/2012 10:33 AM, Brad Roberts wrote:
> Looks simple enough.  I'll try to get it done tonight.
> 
> On 9/4/2012 10:23 AM, Robert Clipsham wrote:
>> Hi all,
>>
>> I don't know if you've seen this, but I thought I'd let you know:
>>
>> https://github.com/blog/1227-commit-status-api
>>
>> It does a similar job to what the auto-tester GreaseMonkey scripts already do but it's built into GitHub and modifies the merge button to make it more obvious. Could be worth looking into adding to the auto-tester at some point.
>>
>> I would assume at some point they'll build on this to allow pull requests to be triaged based on the information.
>>
>> -- 
>> Robert
> 
> _______________________________________________
> dmd-internals mailing list
> dmd-internals@puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals
> 

_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

issue 2: the api only allows for a single status, which makes multi-platform results a bit interesting.  The obvious answer is to union the results together.  Race conditions will make that a little interesting to get right.

issue 3:  Each status update also updates the last updated date.  The end result being that last updated will roughly == last time the pull test was tested.  I don't like that, personally.  I've sent mail to github support to see if they have any plans to make that optional or at least separate.

On the other hand.. kinda nifty:
  https://github.com/D-Programming-Language/dmd/pull/1097

Also missing, no indication of merge-ability shows up on the pull list page, yet.  Presumably something they'll expand on over time.

On 9/4/2012 10:43 AM, Brad Roberts 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.
> 
> On 9/4/2012 10:33 AM, Brad Roberts wrote:
>> Looks simple enough.  I'll try to get it done tonight.
>>
>> On 9/4/2012 10:23 AM, Robert Clipsham wrote:
>>> Hi all,
>>>
>>> I don't know if you've seen this, but I thought I'd let you know:
>>>
>>> https://github.com/blog/1227-commit-status-api
>>>
>>> It does a similar job to what the auto-tester GreaseMonkey scripts already do but it's built into GitHub and modifies the merge button to make it more obvious. Could be worth looking into adding to the auto-tester at some point.
>>>
>>> I would assume at some point they'll build on this to allow pull requests to be triaged based on the information.
>>>
>>> -- 
>>> Robert
>>
>> _______________________________________________
>> dmd-internals mailing list
>> dmd-internals@puremagic.com
>> http://lists.puremagic.com/mailman/listinfo/dmd-internals
>>
> 

_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

On 4 September 2012 19:43, Brad Roberts <braddr@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.
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

On 9/6/2012 12:26 AM, Don Clugston wrote:
> On 4 September 2012 19:43, Brad Roberts <braddr@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?


_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

On 6 September 2012 09:31, Brad Roberts <braddr@puremagic.com> wrote:
> On 9/6/2012 12:26 AM, Don Clugston wrote:
>> On 4 September 2012 19:43, Brad Roberts <braddr@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).
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

On 9/6/2012 2:55 AM, Don Clugston wrote:
> On 6 September 2012 09:31, Brad Roberts <braddr@puremagic.com> wrote:
>> On 9/6/2012 12:26 AM, Don Clugston wrote:
>>> On 4 September 2012 19:43, Brad Roberts <braddr@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


_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals

The changes aren't quite in production yet but it's about 95% ready.  Here's three example pull requests with their statuses set as of a little while ago:

passed:
  https://github.com/D-Programming-Language/phobos/pull/773

work in progress:
  (since finished, but the status hasn't been changed to reflect that yet)
  https://github.com/D-Programming-Language/phobos/pull/768

work complete, with failures:
  https://github.com/D-Programming-Language/phobos/pull/750

It will also show "Pending: #" if there's any left that haven't started yet.  I don't have one of those setup right now.

Before I turn this on.. does anyone strongly object to the last updated field becoming meaningless?  The github guys aren't likely to change the current behavior that a status update is an event that causes the date to move forward.  If I don't hear any major objections, I'm going to turn it on for all of the phobos builds late tomorrow night or saturday some time.

Any thoughts on the descriptive text?  The part that's determined by the api is what's between the dash and the open paren around Details.

Later,
Brad
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals