June 04, 2013
On Tuesday, June 04, 2013 09:52:57 Robert wrote:
> Pull requests should be closed by maintainers if the pull request won't get accepted even if improved quality wise or if the submitter is no longer available for making it ready, but not just because they are old.

I think that the problem is more that the Phobos maintainers are busy and don't necessarily do a good job as a group of reviewing pull requests in general, let alone making sure that each and every pull request gets reviewed and processed in a reasonable time frame. It's something that we need to work on. I think that it's rarely the case that a particular pull request has no comments on it because it's undesirable. It just hasn't been looked at yet like it should have been yet. If it were truly undesirable, it would at minimum have comments saying so if not have been closed.

- Jonathan M Davis
June 04, 2013
On Tuesday, 4 June 2013 at 17:47:36 UTC, Jonathan M Davis wrote:
> On Tuesday, June 04, 2013 09:52:57 Robert wrote:
>> Pull requests should be closed by maintainers if the pull request won't
>> get accepted even if improved quality wise or if the submitter is no
>> longer available for making it ready, but not just because they are old.
>
> I think that the problem is more that the Phobos maintainers are busy and
> don't necessarily do a good job as a group of reviewing pull requests in
> general, let alone making sure that each and every pull request gets reviewed
> and processed in a reasonable time frame. It's something that we need to work
> on. I think that it's rarely the case that a particular pull request has no
> comments on it because it's undesirable. It just hasn't been looked at yet
> like it should have been yet. If it were truly undesirable, it would at
> minimum have comments saying so if not have been closed.
>
> - Jonathan M Davis

Truth. I've had a pull closed in less than an hour once. On the other hand, I have one which has been open for about 7 months now.

It's all a balance of importance vs resource/cost ratio, and reviewer interest. I've closed a few of my own ("valid") pulls myself: they weren't bad, just that I judge reviewer times would be better spent on more important things.

It's a bit frustrating at times, but I find it more than balances when you realize that you actually *get* to participate at all, or even partake in specs discussion...

June 04, 2013
On Sun, 02 Jun 2013 22:54:51 -0700, Jonathan M Davis <jmdavisProg@gmx.com> wrote:

> On Sunday, June 02, 2013 22:49:08 Adam Wilson wrote:
>> I'll see if I can't give you a stab at an explanation. Note that I did not
>> write the Auto-Tester, that is the work of Brad Anderson.
>
> Brad Roberts actually, with some help from Daniel Murphy (who created the pull
> tester), and I don't know who else.
>
> - Jonathan M Davis

I keep missing this up. It doesn't help that I was talking to Brad Anderson (eco) on IRC. Sorry Brad Roberts!

-- 
Adam Wilson
IRC: LightBender
Project Coordinator
The Horizon Project
http://www.thehorizonproject.org/
June 04, 2013
On 6/2/2013 10:49 PM, Adam Wilson wrote:
> Note that I did not write
> the Auto-Tester, that is the work of Brad Anderson.

Brad Roberts

June 05, 2013
On Tue, 04 Jun 2013 16:02:01 -0700, Walter Bright <newshound2@digitalmars.com> wrote:

> On 6/2/2013 10:49 PM, Adam Wilson wrote:
>> Note that I did not write
>> the Auto-Tester, that is the work of Brad Anderson.
>
> Brad Roberts
>

*sigh* I've lost count of how many times I've made that goof. Sorry Mr. Roberts!

-- 
Adam Wilson
IRC: LightBender
Project Coordinator
The Horizon Project
http://www.thehorizonproject.org/
June 05, 2013
On 06/03/2013 07:49 AM, Adam Wilson wrote:
> - Pull requests are tested by cloning [DMD/DRuntime/Phobos] master and merging
> the pull into it. This ensures a clean pull against master.
> - The AT must restart pull request testing every time code is merged into
> DMD/DRuntime/Phobos because of the above testing strategy.
> - If many pulls are merged into [DMD/DRuntime/Phobos] master in a given period
> of, for example a day, the AT will restart prior to completing a full test pass.
> - With the current number of open pulls the AT can complete a full testing pass
> in roughly 8 hours.
> - Each pull takes a rough average of 15 minutes to test.

What I don't understand is how the number of "pending" tests fluctuates.  Not too long ago a pull request of mine to Phobos had passed 9, with 1 pending.  Now it's pending 10.  I have not pushed any changes in the interim ... !

June 05, 2013
On Wednesday, 5 June 2013 at 16:22:19 UTC, Joseph Rushton Wakeling wrote:
> On 06/03/2013 07:49 AM, Adam Wilson wrote:
>> - Pull requests are tested by cloning [DMD/DRuntime/Phobos] master and merging
>> the pull into it. This ensures a clean pull against master.
>> - The AT must restart pull request testing every time code is merged into
>> DMD/DRuntime/Phobos because of the above testing strategy.
>> - If many pulls are merged into [DMD/DRuntime/Phobos] master in a given period
>> of, for example a day, the AT will restart prior to completing a full test pass.
>> - With the current number of open pulls the AT can complete a full testing pass
>> in roughly 8 hours.
>> - Each pull takes a rough average of 15 minutes to test.
>
> What I don't understand is how the number of "pending" tests fluctuates.  Not
> too long ago a pull request of mine to Phobos had passed 9, with 1 pending.  Now
> it's pending 10.  I have not pushed any changes in the interim ... !

Every commit to master causes retesting of all pull requests.
June 05, 2013
On Wednesday, June 05, 2013 18:22:09 Joseph Rushton Wakeling wrote:
> What I don't understand is how the number of "pending" tests fluctuates. Not too long ago a pull request of mine to Phobos had passed 9, with 1 pending. Now it's pending 10. I have not pushed any changes in the interim ... !

Probably because another commit was merged into dmd, druntime, or Phobos, so it had to start again.

- Jonathna M Davis
June 05, 2013
On 06/05/2013 07:06 PM, Jonathan M Davis wrote:
> Probably because another commit was merged into dmd, druntime, or Phobos, so it had to start again.

Ahh, OK.  Thanks to you and Brad for the clarification! :-)

1 2
Next ›   Last »