www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - does it scale to have 1 person approve of all phobos additions?

reply Timothee Cour <thelastmammoth gmail.com> writes:
https://wiki.dlang.org/Contributing_to_Phobos mentions:

 Smaller additions like individual functions can be merged directly after
 andralex approves



The arguments for having all changes go through one person have been
presented here [1].

However this is how I see things:

* if phobos is supposed to be batteries included
(https://forum.dlang.org/post/mfrv29$t21$1 digitalmars.com), it should
be able to grow fast; looking at past changelogs, this has not been
the case, each release only comes with a handful additions to phobos
at most.

* let's look at the D survey answers to the question 'what went wrong'
while contributing code to dlang on github:
https://github.com/wilzbach/state-of-d-2018/blob/master/13c:%20What%20went%20wrong%3F
45 answers out of 69 mentioned the review process was too slow and
PR's lingering forever after all comments are addressed. A common
theme is PR lingers while waiting for approval from leadership [2]

* This creates a vicious cycle which diminishes number of contributors
since PR review is so inefficient; as a result a bug fix / useful
addition never gets merged.

* I'm not sure if there are many examples of large projects where
every addition/symbol overload has to be approved by a single person;
this would be more bearable if response time was fast; however as
noted in survey answers, response time is often in weeks/months,
sometimes years.

pinging by email also doesn't always work: here was the response I got:

 Thanks. This will need to wait - we're busy with DConf for the time being.


if one is unavailable, one should delegate


Given all this, my recommendation would be for PR's to be merged after
all checks have passed and it was approved by 2 committers.

---

[1] https://forum.dlang.org/post/ncp5g8$20hr$1 digitalmars.com

 I just asked for a stdlib addition to be pulled back at
https://github.com/D-Programming-Language/phobos/pull/4025. Such decisions are
difficult because the risk is them to be interpreted as stifling creativity.
That is not the case. The only reason for all library additions to go through
one person/small group is to preserve a cohesive vision and style. At the
opposite end, nobody wants a library that's a mishmash of styles and
approaches, even if that includes some of theirs. Please make sure I know of
library additions. I've been on vacation and even when I'm not I can't monitor
and review all library work - it would be a full-time job that wouldn't leave
me time for anything else. Please just email me directly related pull requests.
I always tend to my email.


[2] here are some illustrative answers:
* PR's linger forever after all comments have been addressed; not
enough committers create a bottleneck from andrei/walter (not enough
trust/delegation); style issues are a waste of time (we should use
tooling instead); negative bias towards 'small' improvements
* Andrei Alexandrescu had way too much influence in areas outside his
core competency, too many bad decisions made for
religious/philosphical reasons relying on the "sufficently powerful
compiler" fallacy and appeal to authority
* PRs have a tendency to grow stale, especially when waiting on a
response from Walter or Andrei.
* There is sometimes a tendency for PRs to languish - sometimes for
years, particularly if there's any disagreement or if it requires
input from Walter or Andrei. Obviously, that doesn't always happen,
but it's not entirely uncommon either.
* Community is very negative.  Leadership seems very unengaged.  There
is not enough delegation/trust.
* I'd say "The whole process was an uphill battle", but that's a huge
understatement. Endless "perfect is the enemy of good".  Endless
pushback and arguing on whether things should even be done at all
(especially from MartinNowak - nothing personal, and no offense, but
that one alone doubles the amount of arguing that needs done to get
anywhere, will object to seemingly anything, and frequently just leads
the debate in circles). And long periods with no response.
* Still no response to my pull request after fixing it 2 weeks after
the initial feedback (more than a few months of waiting now)
* It's very hard to get significant improvements to D's weakest areas
accepted, because of concerns about breaking changes and/or excessive
complexity of proposed solutions. As a result, broken stuff just stays
broken.
* Mixed experience. Sometimes too much of a "don't touch our project"
attitude. (advice: give the contributing developer some ownership of
the project. Yes that means making compromises on your own opinions)
Mar 20 2018
next sibling parent reply Meta <jared771 gmail.com> writes:
Does it make sense? In my opinion, no, but according to Andrei be 
has tried being less hands-on before and it resulted in 
measurably worse quality code in Phobos; thus, he re-established 
himself as the gatekeeper. I agree that it doesn't scale and 
think that at this point, it's probably actively hurting Phobos 
because a lot of good work sits for so long and eventually 
becomes abandoned.

On the other hand, it could become much worse for Phobos if he 
was entirely hands off and delegated its shepherding to a larger 
group of core contributors. A balance has to be struck 
somewhere... Maybe a hypothetical group like this needs to be 
trained by Andrei such that he can trust them to properly guide 
Phobos' development, and will only come to him with the really 
big, important stuff.

By the same measure, I feel that Walter is becoming a bottleneck 
on dmd development and maybe a similar solution is necessary.
Mar 20 2018
next sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Tue, Mar 20, 2018 at 10:56:00PM +0000, Meta via Digitalmars-d wrote:
[...]

 On the other hand, it could become much worse for Phobos if he was
 entirely hands off and delegated its shepherding to a larger group of
 core contributors. A balance has to be struck somewhere... Maybe a
 hypothetical group like this needs to be trained by Andrei such that
 he can trust them to properly guide Phobos' development, and will only
 come to him with the really big, important stuff.


I've brought this up before.  IMO, there is insufficient trust given to
contributors, usually for good reasons, but with the bad consequence
that things keep getting held up.  The only viable solution that I can
see is like you said: Andrei needs to select a group of people he
trusts, and train them so that they will make decisions to his liking.
Then trust them to make the right decisions and let them do their job.

This model has been proven to work, e.g., in the Linux kernel, where
even though Linus still has the final say on things, most of the work is
done by trusted delegates, each of whom is responsible for specific
areas that they have expertise in (and where Linus trusts their
judgment), and not everything requires personal attention from Linus.
Without that delegation, Linux would have remained a hobby project, and
would never have become what it is today.

A similar approach could be adopted for Phobos. In some sense it
somewhat already has, but it could be increased.  For example, Jonathan
Davis is pretty much the go-to person for decisions on std.datetime, and
Dmitry Olshansky is the go-to person for std.regex and std.uni.  There
are a few more, but those are the most obvious ones that come to mind.
If more could be involved in more areas, it would help things.


On the flip side, though, I think it's unfair to put all the blame on
Andrei.  The fact of the matter is that Phobos is big -- far too big
given the number of active contributors.  IME, 90% of PR activity is
centered around a small number of core modules like std.algorithm,
std.range, std.stdio, std.format, std.typecons, and multiple people feel
competent enough to review PRs in these areas.  So PRs in these modules
generally get well-reviewed and merged within a relatively reasonable
timeframe.

But outside of this core group of modules, people are less comfortable
to review -- I, for one thing, would tend to avoid reviewing PRs to
modules that I don't use, simply because I don't use it and therefore
wouldn't know off-hand what might or might not be a good decision for
that module.  The problem is that almost every other Phobos dev also
feels the same way.  And given the sheer size of Phobos, there are a
large number of such modules that most of us are afraid to touch because
we don't feel confident enough to oversee it.  And whoever wrote the
original code has long since moved on, or is otherwise too busy, so PRs
will just sit there unattended.

I don't have any good solutions for how to fix this problem, besides
what everyone is probably already tired of hearing. (Start contributing,
develop a thick skin and a stubborn persistence to keep pushing things
until they get through. Make yourself heard. Make yourself one of the
trusted delegates so that you can actually do something about all this.)



 By the same measure, I feel that Walter is becoming a bottleneck on
 dmd development and maybe a similar solution is necessary.


I haven't been too involved with dmd development, but AFAICT this isn't
strictly true.  I've had a few PRs merged into dmd without going through
Walter.  Though it is true that the dmd PR queue needs some serious
love.  Losing Kenji a few years ago was a silent, but major loss to D,
esp. to dmd development.  To this day I still miss his seemingly
tireless stream of dmd bugfixes and improvements.  :-(

Having said that, though, some of the older dmd PRs do involve fairly
tricky and complicated changes, and it should not be a surprise that
it's hard to make a decision on those issues.  This has to be taken into
consideration, beyond just looking at the outward size of the PR queue.

Furthermore, a good number of dmd PRs *are* being merged every day, so I
think it's a bit unfair to accuse Walter of slowing things down, much as
I agree that the process could be improved.


T

-- 
If lightning were to ever strike an orchestra, it'd always hit the conductor
first.
Mar 20 2018
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 03/20/2018 06:56 PM, Meta wrote:

 Does it make sense? In my opinion, no, but according to Andrei be has 
 tried being less hands-on before and it resulted in measurably worse 
 quality code in Phobos; thus, he re-established himself as the 
 gatekeeper. I agree that it doesn't scale and think that at this point, 
 it's probably actively hurting Phobos because a lot of good work sits 
 for so long and eventually becomes abandoned.
 
 On the other hand, it could become much worse for Phobos if he was 
 entirely hands off and delegated its shepherding to a larger group of 
 core contributors. A balance has to be struck somewhere... Maybe a 
 hypothetical group like this needs to be trained by Andrei such that he 
 can trust them to properly guide Phobos' development, and will only come 
 to him with the really big, important stuff.


Thanks for this comment (which is eerily accurate), and thanks Timothee 
for raising the matter.

It is an ongoing burden to be the decider on new API additions to 
Phobos; indeed I have taken this responsibility because I have attempted 
to relinquish it in the past, with negative results. It is definitely 
not something that I prefer or enjoy, and am permanently on the lookout 
for people with similar design sensibilities to share the burden with. 
The door is open, if not kicked off its hinges. Please take note!

That said, the question of scalability is a bit misplaced. API additions 
to Phobos are rare and long-lasting; it is entirely appropriate to let 
them ripe a little. In contrast, various improvements to Phobos - over 
100 of them - only need good reviews, and are obviously bottlenecked by 
our general lack of reviewers. That's our real bottleneck. It seems 
appropriate to ask the question why we'd ask for acceleration of API 
additions without improving response for other work.

I just reviewed https://github.com/dlang/phobos/pull/6178. As I'd 
expected, it's good work - which is exactly the matter. Good work in a 
submission means most review work. It's not bad work, which can be 
easily rejected. And it's not brilliant work, which can be easily 
accepted. The PR has bugs and quality issues that any reviewer could 
find and provide feedback on. It's not in the state where it's 
bottlenecked by just a stamp of approval.

Naming is a matter I wanted to defer having a debate on. We should call 
the facility staticArray to prevent an imaginary conversation like this:

Q: "So I have a range here, how do I get an array from it?"

A: "Easy, just append .array to it and you're done."

Q. "Cool! Now I need a static array. Wait! Don't tell me, don't tell 
me... staticArray is what I should look for!"

A: "Um, no, sorry. That's called asStatic."

Besides, [1,2].asStatic may be guessed right by a reader, but 
myrange.asStatic!2 most likely not.


Thanks,

Andrei
Mar 21 2018
parent reply Meta <jared771 gmail.com> writes:
On Wednesday, 21 March 2018 at 21:25:55 UTC, Andrei Alexandrescu 
wrote:

 On 03/20/2018 06:56 PM, Meta wrote:

 Does it make sense? In my opinion, no, but according to Andrei 
 be has tried being less hands-on before and it resulted in 
 measurably worse quality code in Phobos; thus, he 
 re-established himself as the gatekeeper. I agree that it 
 doesn't scale and think that at this point, it's probably 
 actively hurting Phobos because a lot of good work sits for so 
 long and eventually becomes abandoned.
 
 On the other hand, it could become much worse for Phobos if he 
 was entirely hands off and delegated its shepherding to a 
 larger group of core contributors. A balance has to be struck 
 somewhere... Maybe a hypothetical group like this needs to be 
 trained by Andrei such that he can trust them to properly 
 guide Phobos' development, and will only come to him with the 
 really big, important stuff.


 Thanks for this comment (which is eerily accurate), and thanks 
 Timothee for raising the matter.

 It is an ongoing burden to be the decider on new API additions 
 to Phobos; indeed I have taken this responsibility because I 
 have attempted to relinquish it in the past, with negative 
 results. It is definitely not something that I prefer or enjoy, 
 and am permanently on the lookout for people with similar 
 design sensibilities to share the burden with. The door is 
 open, if not kicked off its hinges. Please take note!

 That said, the question of scalability is a bit misplaced. API 
 additions to Phobos are rare and long-lasting; it is entirely 
 appropriate to let them ripe a little. In contrast, various 
 improvements to Phobos - over 100 of them - only need good 
 reviews, and are obviously bottlenecked by our general lack of 
 reviewers. That's our real bottleneck. It seems appropriate to 
 ask the question why we'd ask for acceleration of API additions 
 without improving response for other work.

 I just reviewed https://github.com/dlang/phobos/pull/6178. As 
 I'd expected, it's good work - which is exactly the matter. 
 Good work in a submission means most review work. It's not bad 
 work, which can be easily rejected. And it's not brilliant 
 work, which can be easily accepted. The PR has bugs and quality 
 issues that any reviewer could find and provide feedback on. 
 It's not in the state where it's bottlenecked by just a stamp 
 of approval.

 Naming is a matter I wanted to defer having a debate on. We 
 should call the facility staticArray to prevent an imaginary 
 conversation like this:

 Q: "So I have a range here, how do I get an array from it?"

 A: "Easy, just append .array to it and you're done."

 Q. "Cool! Now I need a static array. Wait! Don't tell me, don't 
 tell me... staticArray is what I should look for!"

 A: "Um, no, sorry. That's called asStatic."

 Besides, [1,2].asStatic may be guessed right by a reader, but 
 myrange.asStatic!2 most likely not.


 Thanks,

 Andrei


Thanks. I stewed on this for a few days, and now it's 3 AM and I 
wrote a long reply but deleted it. I agree with most of what you 
you've said, and am progressively agreeing less with what I said. 
Mostly, I'm just frustrated and don't really have any good 
solutions but the PR queue keeps growing. I'll go review 
something.
Mar 23 2018
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 03/24/2018 02:29 AM, Meta wrote:

 I'll go review something.


That's the spirit! Thanks!! -- Andrei
Mar 24 2018
prev sibling next sibling parent reply bachmeier <no spam.net> writes:
On Tuesday, 20 March 2018 at 22:09:18 UTC, Timothee Cour wrote:

[...]

No, it doesn't scale, and years of evidence have demonstrated 
that.

I see no way that this will change, and because delegation is off 
the table, the only realistic way for the language to progress is 
to put as much as possible into libraries. From time to time I 
see posts saying more should be added to Phobos, but the reality 
is that stuff should be removed from Phobos.
Mar 20 2018
parent Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
On Tuesday, March 20, 2018 22:58:44 bachmeier via Digitalmars-d wrote:

 On Tuesday, 20 March 2018 at 22:09:18 UTC, Timothee Cour wrote:

 [...]

 No, it doesn't scale, and years of evidence have demonstrated
 that.

 I see no way that this will change, and because delegation is off
 the table, the only realistic way for the language to progress is
 to put as much as possible into libraries. From time to time I
 see posts saying more should be added to Phobos, but the reality
 is that stuff should be removed from Phobos.


Well, the kind of improvements that potentially have Andrei as a bottleneck
are generally small. We're talking a function or two at a time here. And the
pros and cons of that situation can definitely be debated, but in terms of
adding anything significant to Phobos, Andrei isn't generally a bottleneck.
Adding modules means going through the Phobos review process, which is an
entirely different beast and one that almost no one seems to want to go
through - or in some cases, part of the problem may be that few enough
attempts have been made recently to include new modules that potential
contributors don't even realize how that works. Either way, we don't have
people coming forward with major contributions to Phobos. All we're dealing
with is a bunch of smaller improvements to Phobos, and while those are
definitely valuable, they're also frequently not the sort of thing where
there's debate about putting it in Phobos vs a third party library - or at
least, you wouldn't create a third party library for that functionality; at
most, it would just fit into an existing library that contains a variety of
functionality, whereas if you're talking about adding large enough pieces of
functionality to add a new module, creating a library for that would
probably make sense.

So, we're really dealing with two separate issues when we start talking
about Andrei being a bottleneck and talking about making major contributions
to Phobos where maybe those contributions should just be third party
libraries.

And as for putting major, new functionality into Phobos, we probably do need
to do a better job of that than we have given that we have some older
modules that we'd like to replace that haven't been, but for most stuff,
there's really no reason that it needs to be in Phobos as opposed to on
code.dlang.org other than the fact that having it in Phobos gives it more
exposure and makes it easier for folks to find. Some of it could go into
Phobos, and there is a debate to be had there with any particular piece of
functionality, but it can also just be on code.dlang.org and work just fine.
In either case, it needs folks to write it and maintain it.

Arguably, if we're looking to improve the situation with major pieces of
functionality being available in D, we need to figure out how to improve the
situation with code.dlang.org and encourage solid contributions there. Some
of that could and should end up in Phobos, but the vast majority of useful
code out there is not going to be in the standard library, no matter what
language you're talking about. So, no matter where the line is exactly for
what should and shouldn't be in Phobos, finding ways to improve
code.dlang.org and encourage contributions there may very well be more
valuable in the long run.

In any case, overall, I'm inclined to think that some of what's managed to
make it into Phobos and a number of the things that some folks push to have
in the standard library (e.g. http stuff or database stuff) should probably
just be third party libraries rather trying to insist that Phobos include
everything and the kitchen sink. I think that our experience thus far has
shown that it's fairly difficult to have everything and the kitchen sink in
Phobos and have it be well-maintained. The issues that led to Andrei
deciding to insist on approving every symbol being added to Phobos are just
part of that, but regardless, in order to have more of a kitchen sink, we
would probably not only need to make adjustments to how we handle PRs for
Phobos, but we would need more people to come forward with major
contributions for the review queue.

In the end, the biggest thing we need is for useful libraries to be written,
be available, and be discoverable by those looking for them. Whether that
code is in Phobos or not doesn't matter in that respect. code.dlang.org is
getting there, but as has been discussed before, there are improvements that
should be made to it to help with all of that, and ultimately, we need folks
who are willing to spend their time writing good code and making it
available, whether we're talking about code.lang.org, Phobos, dmd, or
whatever.

- Jonathan M Davis
Mar 20 2018
prev sibling next sibling parent Tony <tonytdominguez aol.com> writes:
I have never used DUB, but as I understand it, it will 
automatically bring down modules that are stored in gitub or two 
other git hosts (but not SourceForge for some reason). With that 
kind of functionality, it seems that inclusion in the standard 
library becomes much less important for a library. Rather than 
being included into Phobos, modules could be sanctioned/blessed 
in some fashion by dlang.org beyond their inclusion at 
code.dlang.org . Such as having their documentation on dlang.org 
(or wiki.dlang.org with a link to the wiki page on a dlang.org 
page that is for listing "sanctioned modules" or "semi-official 
modules").
Mar 20 2018
prev sibling parent reply John Belmonte <john neggie.net> writes:
FWIW, my dmd bug fix PR is getting languish-y.

    https://github.com/dlang/dmd/pull/8051

Ideally a good bug fix shouldn't sit around for a week.  Why I'd 
call this one good:

     * in addition to reported bug (struct initializer incorrectly 
parsed as function literal), a read of the code uncovered 
converse as well (function literal incorrectly parsed as struct 
literal).  PR fixes both and adds test cases.

     * documents additional ambiguous parser cases, and why we 
resolve the ambiguous cases as we do, and how to work around it.  
Added unit tests to confirm handling of ambiguous cases.

What went well:  appveyor flagged a regression which sent me back 
to the drawing board, resulting in a much better fix

Regards,
--John
Apr 01 2018
parent Joakim <dlang joakim.fea.st> writes:
On Sunday, 1 April 2018 at 08:07:09 UTC, John Belmonte wrote:

 FWIW, my dmd bug fix PR is getting languish-y.

    https://github.com/dlang/dmd/pull/8051

 Ideally a good bug fix shouldn't sit around for a week.  Why 
 I'd call this one good:

     * in addition to reported bug (struct initializer 
 incorrectly parsed as function literal), a read of the code 
 uncovered converse as well (function literal incorrectly parsed 
 as struct literal).  PR fixes both and adds test cases.

     * documents additional ambiguous parser cases, and why we 
 resolve the ambiguous cases as we do, and how to work around 
 it.  Added unit tests to confirm handling of ambiguous cases.

 What went well:  appveyor flagged a regression which sent me 
 back to the drawing board, resulting in a much better fix

 Regards,
 --John


I'd say your PR has been handled well: you received feedback 
pretty quickly and it has been approved by a member of the core 
team.  It's likely only been sitting unmerged in a waiting period 
to give others time to weigh in, as not everybody has time to 
check PRs regularly.
Apr 01 2018