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
