On Sat, 12 Nov 2011 07:08:27 +0200, Daniel Murphy <yebblies at gmail.com> wrote:

> There already is a tester available for all open pull requests:
> http://yebblies.com/results/
> Although it's linux32 only, and not particularly fast, pull requests
> that fail on there should never be pulled.

Have you thought about using the GitHub API and make it post comments to the respective pull requests for each "status change" (new pull request, or existing pull request started failing)? This would provide immediate feedback to the maintainers and to the pull request author.

-- 
Best regards,
  Vladimir                            mailto:vladimir at thecybershadow.net

On 11/11/2011 7:28 PM, Walter Bright wrote:
>
>
> On 11/11/2011 11:12 AM, Andrei Alexandrescu wrote:
>>
>> I think that's a terrific offer that we should take. Thanks, Brad! Also it's not about an either-or choice; improvements in both automated testing AND efficient testing should pay off handsomely.
>>
>
> Would such a tester be available for those submitting patches so they can run the autotester *before* an attempt is made to merge?

They already are!  The auto-tester does nothing beyond running the dmd, druntime, and phobos unit tests.

On Friday, November 11, 2011 21:15:06 Brad Roberts wrote:
> On 11/11/2011 7:28 PM, Walter Bright wrote:
> > On 11/11/2011 11:12 AM, Andrei Alexandrescu wrote:
> >> I think that's a terrific offer that we should take. Thanks, Brad! Also it's not about an either-or choice; improvements in both automated testing AND efficient testing should pay off handsomely.>
> > Would such a tester be available for those submitting patches so they can run the autotester *before* an attempt is made to merge?
> 
> They already are!  The auto-tester does nothing beyond running the dmd, druntime, and phobos unit tests.

No, the autotester does not run them _before_ a merge is done. It runs them _after_. So, it's quite good at telling you whether the code in master repositories for dmd, druntime, and Phobos is currently broken, but it doesn't tell you whether the code in _pull requests_ is broken, which appears to be what Walter is looking for. Daniel Murphy's autotester does that, but not the primary one - not unless you've changed something that I don't know about.

- Jonathan M Davis

On 11/11/2011 9:18 PM, Jonathan M Davis wrote:
> On Friday, November 11, 2011 21:15:06 Brad Roberts wrote:
>> On 11/11/2011 7:28 PM, Walter Bright wrote:
>>> On 11/11/2011 11:12 AM, Andrei Alexandrescu wrote:
>>>> I think that's a terrific offer that we should take. Thanks, Brad! Also it's not about an either-or choice; improvements in both automated testing AND efficient testing should pay off handsomely.>
>>> Would such a tester be available for those submitting patches so they can run the autotester *before* an attempt is made to merge?
>>
>> They already are!  The auto-tester does nothing beyond running the dmd, druntime, and phobos unit tests.
>
> No, the autotester does not run them _before_ a merge is done. It runs them _after_. So, it's quite good at telling you whether the code in master repositories for dmd, druntime, and Phobos is currently broken, but it doesn't tell you whether the code in _pull requests_ is broken, which appears to be what Walter is looking for. Daniel Murphy's autotester does that, but not the primary one - not unless you've changed something that I don't know about.
>
> - Jonathan M Davis

Daniel's is before, mine is after.. both run the same set of tests (except that the pre-merge tester is only linux 32 and doesn't run every argument combination for the dmd tests).

On Friday, November 11, 2011 21:19:47 Brad Roberts wrote:
> On 11/11/2011 9:18 PM, Jonathan M Davis wrote:
> > On Friday, November 11, 2011 21:15:06 Brad Roberts wrote:
> >> On 11/11/2011 7:28 PM, Walter Bright wrote:
> >>> On 11/11/2011 11:12 AM, Andrei Alexandrescu wrote:
> >>>> I think that's a terrific offer that we should take. Thanks, Brad! Also it's not about an either-or choice; improvements in both automated testing AND efficient testing should pay off handsomely.>
> >>> 
> >>> Would such a tester be available for those submitting patches so
> >>> they
> >>> can run the autotester *before* an attempt is made to merge?
> >> 
> >> They already are!  The auto-tester does nothing beyond running the
> >> dmd,
> >> druntime, and phobos unit tests.
> > 
> > No, the autotester does not run them _before_ a merge is done. It runs them _after_. So, it's quite good at telling you whether the code in master repositories for dmd, druntime, and Phobos is currently broken, but it doesn't tell you whether the code in _pull requests_ is broken, which appears to be what Walter is looking for. Daniel Murphy's autotester does that, but not the primary one - not unless you've changed something that I don't know about.
> > 
> > - Jonathan M Davis
> 
> Daniel's is before, mine is after.. both run the same set of tests (except that the pre-merge tester is only linux 32 and doesn't run every argument combination for the dmd tests).

Yes. It just looked like you were saying that yours checked pull requests, which it doesn't. It checks what's already been merged into the main repository.

- Jonathan M Davis

On Friday, November 11, 2011 20:58:16 Jonathan M Davis wrote:
> On Friday, November 11, 2011 19:25:59 Walter Bright wrote:
> > On 11/11/2011 10:50 AM, Brad Roberts wrote:
> > > The bigger issue, imho, is Walter's lack of trust in the automated testers.
> > 
> > Another issue is when I do a pull, and it fails testing. This just happened. Things pretty much grind to a halt when that happens. I wish github had an "unpull" button.
> 
> Git in general doesn't have a way to unpull AFAIK, which would make it rather hard for github to do it.
> 
> Probably the correct way to handle this if you're trying to avoid ever merging in bad pulls is to simply not use the pull button - or at least not use it immediately. If you merge the request in manually into a branch on your local machine, you can test it there.
> 
> For instance, if Kenji had a pull request on a branch named fix, then you could do this (9rnsr is Kenji's github ID):
> 
> git-checkout -b fix
> git-pull git://github.com/9rnsr/dmd fix
> 
> This creates a local branch named fix and pulls in Kenji's changes. You then run the test suite or whatever else you do to verify the fix. After you're sure that they're good, you then either use the button on github or merge the fix branch into your master branch and push that into the master repository.
> 
> But unfortunately, I'm not aware of any way to automate unmerging. If you determined which commits they were, you could revert them one by one, but that's the best that I'm aware of - especially if you don't want to rebase (which would be very bad to do with the main repository, since that would screw up everyone's forks).

And actually, automating this process is exactly what Daniel Murphy's autotester is designed to do. So, if you really want to speed up checking pull requests before merging them in, then you're likely going to want a machine which does that (be it Daniel Murphy's currenty autotester or a new one that you set up).

- Jonathan M Davis

On Sat, 12 Nov 2011 07:15:06 +0200, Brad Roberts <braddr at puremagic.com> wrote:

> They already are!  The auto-tester does nothing beyond running the dmd, druntime, and phobos unit tests.

Your auto-tester also runs on a much larger variety of platforms than is available to most contributors.

-- 
Best regards,
  Vladimir                            mailto:vladimir at thecybershadow.net

On Sat, 12 Nov 2011 06:58:16 +0200, Jonathan M Davis <jmdavisProg at gmx.com> wrote:

> Git in general doesn't have a way to unpull AFAIK, which would make it rather hard for github to do it.

What about reverting the merge commit?

git revert -m 1 <merge-commit-SHA1>

-- 
Best regards,
  Vladimir                            mailto:vladimir at thecybershadow.net

On Saturday, November 12, 2011 07:38:01 Vladimir Panteleev wrote:
> On Sat, 12 Nov 2011 06:58:16 +0200, Jonathan M Davis <jmdavisProg at gmx.com>
> 
> wrote:
> > Git in general doesn't have a way to unpull AFAIK, which would make it rather hard for github to do it.
> 
> What about reverting the merge commit?
> 
> git revert -m 1 <merge-commit-SHA1>

That might work. I don't know. I haven't looked at that particular option of revert before, and looking at the man page for git-revert, it looks like it might have some nasty side effects to your ability to merge in the changes after they've been fixed. But that does look like it might be a way to revert the merge if used correctly.

- Jonathan M Davis

On 11/11/2011 8:06 PM, Michel Fortin wrote:
>
> I probably wrote all that in too much details. I hope it is helpful though.

Thanks. This level of detail is what I need.