I've been looking at 6000+ messages about dlang pull requests and their associated chatter during the past couple of days. While at it I pulled a bunch of requests, and by this I'm asking you to join me.

I have accumulated a bunch of insight in short term memory, among which (1) people are happy to work on a variety of things if given just a little vision and guidance; (2) it is more difficult yet more productive in the long run to shepherd a weak submission with good parts to completion, than to dismiss it for its defects; (3) being able to drain the request pipeline effectively is key to D's success.

Please join me in reviewing submissions to https://github.com/D-Programming-Language/. It doesn't matter if you're on the commit team - you only need to be on github, and any meaningful review matters. This is much more important, and offers a lot more leverage, than spending the same time posting on the forum.

After my review binge, Phobos open pull requests are 44, a historical low. Please hop on and let's drain that pipe entirely!


Thanks,

Andrei

On Sunday, 16 March 2014 at 21:46:09 UTC, Andrei Alexandrescu wrote:
>
> Thanks,
>
> Andrei

True dat. Every reviewer helps, and keeping the pipeline low helps. In particular, it creates a sinergy where the better the faster we pull, the faster quality submissions come in, which is awesome.

----

But.... To all the "expert reviewers" out there (myself included): Let's try to be "productive" in our reviews, and try to keep a broader "big picture" view on the pulls.

1° First: Check the design is correct.
2° THEN: Check the implementation is correct.
3° FINALLY: (optionally) Check the style is correct.

Doing this in the wrong order usually means wasted effort, and/or extra useless noise in the review. Furthermore, it can aggravate the original submitter, who went through the effort of fixing every implementation detail, just see his work rejected because of the concept (for example, the "tee" range...).

And go easy on style. The result noise of arguing over style usually outweighs its benefits. If it works, and doesn't break the style rules, then let's just merge and move on (IMO).

On 03/16/2014 10:46 PM, Andrei Alexandrescu wrote:
> I've been looking at 6000+ messages about dlang pull requests and their
> associated chatter during the past couple of days. While at it I pulled
> a bunch of requests, and by this I'm asking you to join me.

Thanks a lot.

> I have accumulated a bunch of insight in short term memory, among which
> (1) people are happy to work on a variety of things if given just a
> little vision and guidance; (2) it is more difficult yet more productive
> in the long run to shepherd a weak submission with good parts to
> completion, than to dismiss it for its defects; (3) being able to drain
> the request pipeline effectively is key to D's success.

I came to the same conclusion, we're currently loosing to many useful work.

On Sunday, 16 March 2014 at 22:10:50 UTC, monarch_dodra wrote:
> But.... To all the "expert reviewers" out there (myself included): Let's try to be "productive" in our reviews, and try to keep a broader "big picture" view on the pulls.
>
> 1° First: Check the design is correct.
> 2° THEN: Check the implementation is correct.
> 3° FINALLY: (optionally) Check the style is correct.
>
> Doing this in the wrong order usually means wasted effort, and/or extra useless noise in the review. Furthermore, it can aggravate the original submitter, who went through the effort of fixing every implementation detail, just see his work rejected because of the concept (for example, the "tee" range...).

Certain members with commit rights are really trigger happy. I always feel like in a rush to point out any and all issues I can see ASAP because otherwise it might be hastily merged.

I'd like to thank you for leaving a PR open for a couple of days before merging after having reviewed it yourself. Even if this practice was widely adopted though, the problem remains with PRs sometimes being merged even when there are still outstanding objections.

> And go easy on style. The result noise of arguing over style usually outweighs its benefits. If it works, and doesn't break the style rules, then let's just merge and move on (IMO).

I agree, but it also needs to be recognized that there is a line between pure style and objective improvements, like clearer variable naming or less sloppily written documentation. I like to think that picking on the latter pays off by raising the bar so that next time, the author will be keener to such issues.

On Sunday, 16 March 2014 at 21:46:09 UTC, Andrei Alexandrescu wrote:
> I have accumulated a bunch of insight in short term memory, among which (1) people are happy to work on a variety of things if given just a little vision and guidance; (2) it is more difficult yet more productive in the long run to shepherd a weak submission with good parts to completion, than to dismiss it for its defects; (3) being able to drain the request pipeline effectively is key to D's success.

I've always thought #2 is especially important when the submitter is a beginner to either D, dmd/druntime/Phobos or the review process; we should aim to foster a culture where the barrier to entry is as low as possible. It's always nice to see more returning submitters.

> After my review binge, Phobos open pull requests are 44, a historical low. Please hop on and let's drain that pipe entirely!

Thanks for the reviews. It would be really awesome if we could get the queue under control. I feel we are heading there.

On 3/16/14, 3:20 PM, Martin Nowak wrote:
> On 03/16/2014 10:46 PM, Andrei Alexandrescu wrote:
>> I have accumulated a bunch of insight in short term memory, among which
>> (1) people are happy to work on a variety of things if given just a
>> little vision and guidance; (2) it is more difficult yet more productive
>> in the long run to shepherd a weak submission with good parts to
>> completion, than to dismiss it for its defects; (3) being able to drain
>> the request pipeline effectively is key to D's success.
>
> I came to the same conclusion, we're currently loosing to many useful work.

It's amazing how this puts discussions in perspective. There's all this hand wringing about what to do to help D move forward, yet this obvious lever has been in front of us all along.

Andrei

On 3/16/14, 3:28 PM, Jakob Ovrum wrote:
> On Sunday, 16 March 2014 at 22:10:50 UTC, monarch_dodra wrote:
> I'd like to thank you for leaving a PR open for a couple of days before
> merging after having reviewed it yourself. Even if this practice was
> widely adopted though, the problem remains with PRs sometimes being
> merged even when there are still outstanding objections.

We can always undo merges.

Andrei

On Sun, 16 Mar 2014 19:47:39 -0400, Andrei Alexandrescu <SeeWebsiteForEmail@erdani.org> wrote:

> On 3/16/14, 3:28 PM, Jakob Ovrum wrote:
>> On Sunday, 16 March 2014 at 22:10:50 UTC, monarch_dodra wrote:
>> I'd like to thank you for leaving a PR open for a couple of days before
>> merging after having reviewed it yourself. Even if this practice was
>> widely adopted though, the problem remains with PRs sometimes being
>> merged even when there are still outstanding objections.
>
> We can always undo merges.

We currently have "auto merge" feature.

What about an "auto merge after 2 days without further objection" feature?

The problem I see with not merging right away is people forget to do it. I would include myself as someone who does that.

Doesn't have to be used in every case, obviously.

-Steve