July 03, 2017
On Sunday, July 02, 2017 00:04:40 H. S. Teoh via Digitalmars-d wrote:
> On Sun, Jul 02, 2017 at 01:56:22AM +0000, Jack Stouffer via Digitalmars-d
wrote:
> > On Tuesday, 27 June 2017 at 01:35:31 UTC, Meta wrote:
> > > On this topic, I feel like we've been falling behind lately in responding to PRs promptly, communicating with submitters on what changes (if any) are needed to get the PR into merge shape, and actually getting stuff merged (this isn't anything new of course). I don't have the data to back me up yet, but I am going to try to gather what I can and make a post about it sometime within the next month. Any ideas or insights are welcome.
> > >
> > > 1. https://github.com/dlang/phobos/pull/5515
> >
> > This is particularly my fault. I haven't been reviewing PRs nearly as much as I was in the past.
>
> I don't think it's specifically your fault... I think we're just short of hands on team Phobos.  I used to do a lot more reviewing / merging too, but this past year I've been unable to because
[...]

I think that that's the story for a number of us. Several years ago, I was reviewing and merging more PRs than anyone, but the past couple of years, I've done relatively little reviewing. I just have a hard time finding time to spend on this sort of thing. And this year, the time since dconf has been particularly bad for me. Some of us do need to find a way to spend more time on this sort of thing, but overall, we simply need more qualified people who have the time and inclination to review PRs. Too often, the qualified folks with the time to do it don't continue to have the that kind of time.

- Jonathan M Davis
July 08, 2017
I thought I'd let everyone know that there has been a whopping 36 PRs merged in the past week (versus 17 opened). We're now sitting at 114 open Phobos PRs. Thanks to the reviewers/mergers who put in the effort to get that number down.
July 09, 2017
On Saturday, 8 July 2017 at 06:05:54 UTC, Meta wrote:
> I thought I'd let everyone know that there has been a whopping 36 PRs merged in the past week (versus 17 opened). We're now sitting at 114 open Phobos PRs. Thanks to the reviewers/mergers who put in the effort to get that number down.

Btw _every_ helping hand in reviewing PRs is very welcome.
It's not very difficult and usually just a "I reviewed this PR and it LGTM" helps to bump the priority.
Otherwise of course, the author should be notified about existing blocking points in his PR.

Since a couple of months, GitHub allows to list all PRs that haven't received a review (yet):

https://github.com/dlang/phobos/pulls?page=3&q=is%3Apr+is%3Aopen+review%3Anone

Also, you can filter out labelled PRs, e.g. all PRs except those that depend on work from the submitter:

https://github.com/dlang/phobos/pulls?utf8=%E2%9C%93&q=is%3Apr%20review%3Anone%20is%3Aopen%20-label%3A%22needs%20work%22%20

The "needs work" label gets automatically removed on a new push.
July 11, 2017
On Sunday, 9 July 2017 at 23:05:05 UTC, Seb wrote:
> On Saturday, 8 July 2017 at 06:05:54 UTC, Meta wrote:
>> I thought I'd let everyone know that there has been a whopping 36 PRs merged in the past week (versus 17 opened). We're now sitting at 114 open Phobos PRs. Thanks to the reviewers/mergers who put in the effort to get that number down.
>
> Btw _every_ helping hand in reviewing PRs is very welcome.
> It's not very difficult and usually just a "I reviewed this PR and it LGTM" helps to bump the priority.
> Otherwise of course, the author should be notified about existing blocking points in his PR.

Yes, 100%. Even if you feel like you don't have the "authority" to review a PR, do it anyway. Even if you don't have merge privileges, the more eyes there are on a PR the larger the chance of somebody seeing something and pointing it out. It also makes it easier for people who *do* have merge privileges if they see that it's already been looked over by a few different people.

> Since a couple of months, GitHub allows to list all PRs that haven't received a review (yet):
>
> https://github.com/dlang/phobos/pulls?page=3&q=is%3Apr+is%3Aopen+review%3Anone
>
> Also, you can filter out labelled PRs, e.g. all PRs except those that depend on work from the submitter:
>
> https://github.com/dlang/phobos/pulls?utf8=%E2%9C%93&q=is%3Apr%20review%3Anone%20is%3Aopen%20-label%3A%22needs%20work%22%20
>
> The "needs work" label gets automatically removed on a new push.

Thanks for getting some more sophisticated automated stuff set up. The more small things we can offload onto machines who don't mind the tedium and don't forget, the better.


1 2 3
Next ›   Last »