One more issue

Mon Feb 16 09:36:57 PST 2015

On 2/16/15 6:33 AM, H. S. Teoh via Digitalmars-d wrote:
> On Sun, Feb 15, 2015 at 07:49:15PM -0800, Andrei Alexandrescu via Digitalmars-d wrote:
>> Just figured that the documentation of package-ized std.algorithm
>> is... not good:
>>
>> http://dlang.org/phobos-prerelease/std_algorithm_package.html
>>
>> It has two seemingly random names in it: "forward" and "SortOutput".
>
> That's because originally I did not know where to put these symbols, and
> rather than delay everything and risking continual merge conflicts, I
> thought it would be more productive to merge first and then submit
> followup PRs to clean up these leftovers. In fact, I did submit a couple
> of PRs afterwards that cleaned up a lot of the stuff from package.d, and
> recently somebody else also submitted a PR to move some of the internal
> implementation details to a private submodule. It's a work in progress.

Top level consideration: no need to get defensive or offended over this. 
Your work is appreciated, and it just so happens this particular issues 
brought to a head a few matters about the review process.

I'll say a few more things below that may be not very pleasant. I count 
on you not deriving an emotional response from them; instead, convert 
this to a good experience that pushes things forward. As one whose most 
work statistically is crap, I certainly empathize. The key here is, 
again, converting this to progress for all involved.

One thought about "forward" and "SortOutput" - first may e.g. go in 
std.functional and the latter belongs to "sorting".

>> Also the table at the beginning uses some odd small fonts for the
>> function names.
>
> Then fix the stylesheet. The macros were changed because now they need
> to link to submodules instead of symbols in the current module.

This is the wrong attitude. Each pull request must leave the place 
better than it found it. You shouldn't leave stuff for others to clean 
up after you.

Hit once, and hit well. May weed not grow again where you hit.

>> With 2.067 this will become the documentation of the most used
>> artifact of the standard library.
>>
>> Is this what we want for 2.067?
>>
>> Apparently the initial splitting work passed review with significant
>> scrutiny: https://github.com/D-Programming-Language/phobos/pull/2879.
>> I'm unclear whether that was the pull that created the various
>> documentation-related issues, or they became worse later. That was
>> definitely the pull that has caused the silent and unacceptable change
>> of name from std_algorithm.html to std_algorithm_package.html.
>
> I have already said that I just copied the example of std.range and
> std.container. There were no guidelines about what the .html filename of
> package.d ought to become, so I did what any reasonable person would do
> -- follow the example of previous modules in the project.

That underlines the importance of precedents. Someone way back when had 
a bad solution for adding packages to std/. It's repetitive and doesn't 
scale. Then in a boiling frog manner, every subsequent package copied 
the bad example. The lesson here is we should not accept bad code, even 
if it seems small and innocuous.

>> Folks, THIS MUST IMPROVE RADICALLY. The long and short of it - and it
>> really pains me to say it - it's shoddy work and shoddy reviewing.
>
> That PR was merged quickly because std.algorithm is one of the most
> frequently touched parts of Phobos, and a major work like splitting it
> into submodules risked continual merge conflicts with incoming PRs. I
> thought it was more productive to merge first and clean up later (which
> I did -- but obviously not good enough), than to spend 6 months arguing
> over nitpicks while the PR bitrots in the queue and becomes unmergeable
> every 2 days. I don't have that kind of time/energy to spend, this
> being, after all, done on my free time rather than me getting paid to do
> this dirty work.

Then one possibility would have been to focus your efforts elsewhere. As 
things are right now:

* There is no added value to D users in splitting algorithm.d into a 
package.

* Even the value to contributors is debatable and definitely low impact.

* The documentation is in shambles.

All told we're looking at a net negative. I understand it can be 
transformed into a positive, but even if executed to perfection it's a 
low-impact artifact. And it's not executed anywhere near perfection.

> And like I said, I did submit a number of followup PRs to clean up a lot
> of things that were leftover from the original PR. I regret that I got
> busy with Real Life and didn't have the chance to cleanup every last bit
> -- I was hoping that, this being a community project, somebody else
> could have picked up the slack without slapping me in the face for
> having not enough time to make everything perfect.

Again please don't take this emotionally. Whatever happened, the balance 
of it is we have a deadline staring us in the face and several grave 
breakages to fix. This all will be useful only if we discuss it in the 
open and learn what it takes to avoid it in the future.

But please don't go "perfect" on me. I was looking for "done".

> In retrospect, perhaps I should never have bothered.

I agree. It's a low impact change. Stop shuffling the deck. Add value. 
For my money I would have been a ton happier if you just finished work 
on groupBy, which _is_ high impact.

>> I'm all for taking a weak contribution and shepherding it toward
>> getting better. I'm speaking personally about myself, and also I hope
>> on behalf of the community. It's the right thing to do.
>>
>> For this to work, we must put the right emphasis on good reviewing and
>> take pride in getting our contributions in top shape.
>>
>> Also, we need to build the right trust that some matter can be
>> delegated appropriately so we don't spread our focus too thin.
>>
>> I appeal to all contributors to improve focus on adding real value to
>> D, and to all reviewers to take good submissions into great
>> submissions.
>>
>> As things are right now, I don't see how we can release 2.067 on time
>> without real help from the entire community.
> [...]
>
> Yes.
>
> It would also help if things like the .html filename policy and other
> such nitpicks were made clear from the beginning instead of being tacked
> on afterwards and retroactively expected of past contributions. It is
> making me rapidly lose interest in contributing any further.

Again, this is the wrong attitude, please wean yourself off of it. 
That's not nitpicking. You changed the name of one high-traffic page on 
dlang.org. There's no need to explain in a guide that that should be 
approached with maximum care.

Your work is appreciated. As I said, statistically most of my work is 
shit. If I let that discourage me, I'd be homeless. Let's convert this 
into something good.

Andrei