Jump to page: 1 25  
Page
Thread overview
January 04
This is in response to some of the frustrations offered in the thread staring with http://forum.dlang.org/post/oeigdutfphxhenexcqsn@forum.dlang.org

I share some of those frustrations, and I've been vocal about such things myself.  However, in the last 2 months, at least in terms of the DMD repository, things may be starting to change for the better.

Recent Promising Progress
-------------------------
A few of us are working hard to revive old PRs and either see them across the finish line, or give them a quick path to closure. 2 months ago the DMD PR queue stood at about 190.  Now it's hovering between 140 and 150.  So, we're getting better; take a look at the graphs here for evidence:  https://auto-tester.puremagic.com/chart.ghtml?projectid=1. Notice the spike in PR submissions in December, and we're still 40 PRs less than we were 2 months ago.

2 months ago, the oldest PR was dated 2012, believe it or not:  https://github.com/dlang/dmd/pull/1145.  Our oldest is now 2013, which is still pretty bad, but it was partially revived recently at https://github.com/dlang/dmd/pull/7503.  Unfortunately, it's probably the most difficult PR we have.

I'll take the risk of arguing that we've widened the review bottleneck recently for straight-forward PRs.  However, for difficult, complex, or controversial PRs, it still looks like we lack contributors with the right skills that are willing volunteer their time, and tolerate some inconveniences.

There has also been a lot of work done on improving infrastructure for testing (e.g. Adding the Appveyor CI for testing Windows) and automating some of the mundane tasks (e.g. https://github.com/dlang/dmd/pull/7507).

A Few Observations About Our Talent
-----------------------------------
IMO, one of the most worrisome issues we have is a talent retention problem (at least for DMD).  It appears many of those with the talent to review and fix difficult compiler bugs are no longer active, or just pop in from time-to-time.  I'm not sure why that is. I know a few have left or don't participate much due to dissatisfaction in the way D is managed.  Others, I think, were working on D as students and now have blossoming careers outside of D to maintain.  I also know of at least one that found Rust to be a better alternative.  But, I can also understand how someone would just grow sick and tired of working on the same thing for months or years.

Walter seems to pop in daily, and occasionally reviews PRs, and his PRs of late are mostly just refactorings rather than fixing difficult bugs.  We desperately need his help on the more difficult issues (e.g. https://github.com/dlang/dmd/pull/7568 and https://github.com/dlang/dmd/pull/7566), but I don't know if he's well-aware of those items and just busy with other things, or if those items are simply not on his radar.  If it's the latter, and it would help, I'll publish a digest periodically.

Andrei pops in unpredictably, and sometimes goes on a PR review rampage.  The infrequency, however, makes it difficult to follow-up and maintain momentum on individual items that need his judgement (e.g. https://github.com/dlang/dmd/pull/7099)

That being said, there are some very talented software engineers active on this forum that are only seldom active in the PR queue. I don't know why that is; perhaps it's simply a matter of time and energy, or perhaps it's frustration.  As others have said, you don't need to have commit access to be a helpful reviewer. And, after showing a willingness to participate, demonstrating your abilities, and earning the confidence of the leadership, you might be asked to join the team, and then you'll really be able to make things happen.

Final Words
-----------
Due to recent changes, if you have a straight-forward, non-controversial PR, it will likely be merged in a week or two; usually less, and sometimes even within a few hours.  If the PR is difficult, complex, or controversial, then it may languish in PR purgatory for some time, unfortunately.  I don't know what can be done about that at this time without skilled, frequently-active talent.

My two-cents,

Mike
January 04
On Thu, 04 Jan 2018 10:34:05 +0000, Mike Franklin wrote:

> This is in response to some of the frustrations offered in the thread staring with http://forum.dlang.org/post/oeigdutfphxhenexcqsn@forum.dlang.org
> 
> I share some of those frustrations, and I've been vocal about such things myself.  However, in the last 2 months, at least in terms of the DMD repository, things may be starting to change for the better.
> 
> Recent Promising Progress -------------------------
> A few of us are working hard to revive old PRs ...


This is an important post. It's easy to make a habit of complaining about things, but it's good to stop and say "but look where we are right now." You do this without acting like all problems are being solved, too. So thank you.

I do think it's important to mention that the improvements you've mentioned came about because some people said "we're going to fix this" rather than wait for someone else to do it. There are problems that can only be resolved by one or two people, but there are many others that anybody take can take care of.
January 04
On Thursday, 4 January 2018 at 10:34:05 UTC, Mike Franklin wrote:
> A few of us are working hard to revive old PRs and either see them across the finish line, or give them a quick path to closure. 2 months ago the DMD PR queue stood at about 190.  Now it's hovering between 140 and 150.

DMD also has at least 30 PRs which have had no action from the author in over a year. There's no reason these should be kept open; they just take up auto-tester resources and lower the signal to noise ratio.

If the author comes back later, they can always ask a maintainer to reopen.
January 04
On Thursday, 4 January 2018 at 17:44:58 UTC, Jack Stouffer wrote:
> DMD also has at least 30 PRs which have had no action from the author in over a year. There's no reason these should be kept open; they just take up auto-tester resources and lower the signal to noise ratio.

FYI: stalled PRs don't take up resources (except they target stable, which is why one of the reasons stable PRs should be merged ASAP). It's due to the auto-tester sorting PRs by their activity.

BTW: For those who haven't seen this, dlang-bot now detects these stalled PRs and for now labels them as "stalled":

https://github.com/dlang-bots/dlang-bot/blob/master/source/dlangbot/cron.d#L60

I'm not sure whether auto-closing them is a good idea, but we definitely need to do sth. about them.
Also one year without any activity is a very long time. Dlang-bot currently uses 90 days though imho that's already very conservative.

Example:

https://github.com/dlang/dmd/pulls?q=is%3Aopen+is%3Apr+label%3Astalled
January 04
On Thursday, 4 January 2018 at 18:09:32 UTC, Seb wrote:
> On Thursday, 4 January 2018 at 17:44:58 UTC, Jack Stouffer wrote:
>> DMD also has at least 30 PRs which have had no action from the author in over a year. There's no reason these should be kept open; they just take up auto-tester resources and lower the signal to noise ratio.
>
> FYI: stalled PRs don't take up resources (except they target stable, which is why one of the reasons stable PRs should be merged ASAP). It's due to the auto-tester sorting PRs by their activity.

They waste resources of folks looking to review stuff that is actual and active.

>
> BTW: For those who haven't seen this, dlang-bot now detects these stalled PRs and for now labels them as "stalled":
>
> https://github.com/dlang-bots/dlang-bot/blob/master/source/dlangbot/cron.d#L60
>
> I'm not sure whether auto-closing them is a good idea, but we definitely need to do sth. about them.
> Also one year without any activity is a very long time.

It’s not like we loose anything, we don’t delete code of the author. So should he/she find time - just create a new PR. Also these willing to dig in the closed PRs are free to do so (it’s just a couple of passes to add a filter to your search).

Personally, I’d say 3 of months of silence usually enough to put off most except for seasoned contributors. Many opensource projects do even worse job but we should look at the best examples.

> Dlang-bot currently uses 90 days though imho that's already very conservative.

Just tagging doesn’t really help, I already can sort by activity.

>
> Example:
>
> https://github.com/dlang/dmd/pulls?q=is%3Aopen+is%3Apr+label%3Astalled


January 04
On Thu, Jan 04, 2018 at 06:50:25PM +0000, Dmitry Olshansky via Digitalmars-d wrote:
> On Thursday, 4 January 2018 at 18:09:32 UTC, Seb wrote:
[...]
> > BTW: For those who haven't seen this, dlang-bot now detects these stalled PRs and for now labels them as "stalled":
> > 
> > https://github.com/dlang-bots/dlang-bot/blob/master/source/dlangbot/cron.d#L60
> > 
> > I'm not sure whether auto-closing them is a good idea, but we
> > definitely need to do sth. about them.
> > Also one year without any activity is a very long time.
> 
> It’s not like we loose anything, we don’t delete code of the author. So should he/she find time - just create a new PR. Also these willing to dig in the closed PRs are free to do so (it’s just a couple of passes to add a filter to your search).

I used to think that it's OK to leave old PRs there, since they don't actively cause harm.  But lately I'm changing my stance.  Having a PR queue that's overly long conveys the wrong image to a potential contributor.  Having PRs that are years old with no resolution is discouraging to look at, and also a source of frustration to the submitter, esp. when the change is not especially complicated.

So nowadays I'm inclined to suggest that if a PR hasn't had any response from the submitter for, say, 2-3 months, it should be closed with a comment to resubmit if the submitter still feels strongly about it. Note that I'm measuring this by activity *from the submitter*; I don't think it's a good idea to close a PR if the submitter is waiting for somebody from the team to respond.  That would send the completely wrong message.

So I'd suggest that if a PR has had no activity from the submitter for, say, 2 weeks, then send a ping. At the 1 month mark, send another ping. At the 2 month mark, close it with a note to reopen if the submitter later decides to work on it again.

If a PR has had no activity from the team for 1 week, then the onus is upon us to do something about it.  I'm not sure what to do when the reason is that none of the active team members feel qualified to handle the proposed change.  For example, if the PR touches some numerical algorithms, I would hesitate to review it, because of lack of expertise in the area.  I guess this is where the lack of consistent talent is a big liability: if the go-to person for a particular area isn't active, or only rarely active, or just too busy, then there's really not much we can reasonably do about it.  Perhaps one small thing that might help is to acknowledge the PR, or take initiative to actively ping the go-to person until he responds.  At the very least, this would send the right message that the team cares about the contribution, but just doesn't have the resources right now to deal with it yet.

Alternatively, and this is just a wild idea off the top of my head, if the go-to person doesn't respond by X days (where X is some fixed number we agree on), a non-expert team member can do a code-level review (i.e., check for obvious glaring problems, check for basic code sanity) and then merge the PR.  Worse comes to worst, the revert button's always there.  This has to be done with care, though.  Definitely should not be done prior to an imminent release, so that we don't introduce problems that are hard to fix later; but perhaps early in the release cycle it might be OK, under the assumption that any obvious problem would manifest itself sometime before the next release, and the PR can be reverted if necessary.  But even then, we might need to mark certain types of changes as off-limits, e.g., if they would be too disruptive, or fundamentally changes the way things work with unknown consequences. Ultimately, though, this is just a crutch in the face of lack of the right talents to do a proper review.  Anyway, this is just a rough idea that may hopefully lead to a better one.


T

-- 
"Computer Science is no more about computers than astronomy is about telescopes." -- E.W. Dijkstra
January 04
On 1/4/2018 2:34 AM, Mike Franklin wrote:
> Walter seems to pop in daily, and occasionally reviews PRs, and his PRs of late are mostly just refactorings rather than fixing difficult bugs.
There's a lot of technical debt I've been trying to fix with that, and nobody else seems willing to do it. For example, fixing the error messages so they make use of color syntax highlighting. It's boring, tedious, unfun work, meaning I get to do it :-)

Another issue is I've had the flu for a while which makes me tired, and then it's best to work on things that don't require much mental energy.

(Yes, I got the flu shot, and the durned thing did not work.)
January 05
On Friday, 5 January 2018 at 03:28:10 UTC, Walter Bright wrote:

> There's a lot of technical debt I've been trying to fix with that, and nobody else seems willing to do it. For example, fixing the error messages so they make use of color syntax highlighting. It's boring, tedious, unfun work, meaning I get to do it :-)

If you have simple, mundane work that needs to be done, make a list and send it my way; you should have my e-mail address.  But please be specific; "fix error messages" is just going to get a reply from me asking questions.

Mike

January 04
On 1/4/18 10:28 PM, Walter Bright wrote:
> (Yes, I got the flu shot, and the durned thing did not work.)

I had a flu shot once in my adult life. Never been sicker. Won't ever get it again.

-Steve
January 04
On 1/4/2018 7:41 PM, Mike Franklin wrote:
> If you have simple, mundane work that needs to be done, make a list and send it my way; you should have my e-mail address.  But please be specific; "fix error messages" is just going to get a reply from me asking questions.

Ok, fix error messages, like you've seen me doing already. To find which files need work,

    grep "error(" *.d

does the trick, turning up:

ctfeexpr.d
dimport.d
dinterpret.d
dscope.d
e2ir.d
expression.d
expressionsem.d
func.d
glue.d
iasm.d
mtype.d
nogc.d
nspace.d
opover.d
optimize.d
s2ir.d

as needing ` `. There's a lot of work just there :-( It's best to resist the urge to do more than just add in ` ` where they are obviously appropriate. Don't make perfection the enemy of better. I've found it best to have only one PR for this at a time, otherwise it's a lot of extra work rebasing.

A longer term goal is to find ways to refactor the code to make maximum use of pure, const, scope, and nothrow.

I appreciate any and all help with this.
« First   ‹ Prev
1 2 3 4 5