www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - One more issue

reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
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". 
Also the table at the beginning uses some odd small fonts for the 
function names.

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.

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.

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.


Thanks,

Andrei
Feb 15 2015
next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/15/15 7:49 PM, Andrei Alexandrescu wrote:
 Just figured that the documentation of package-ized std.algorithm is...
 not good:
[snip] Added https://github.com/D-Programming-Language/phobos/pull/2989, please review. This is important and urgent. Thanks! -- Andrei
Feb 15 2015
prev sibling next sibling parent "Ulrich =?UTF-8?B?S8O8dHRsZXIi?= <kuettler gmail.com> writes:
On Monday, 16 February 2015 at 03:49:14 UTC, Andrei Alexandrescu 
wrote:
 It has two seemingly random names in it: "forward" and 
 "SortOutput". Also the table at the beginning uses some odd 
 small fonts for the function names.
About the font issue: https://github.com/D-Programming-Language/dlang.org/pull/903
Feb 16 2015
prev sibling parent reply "H. S. Teoh via Digitalmars-d" <digitalmars-d puremagic.com> writes:
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.
 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.
 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.
 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. 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. In retrospect, perhaps I should never have bothered.
 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. T -- What do you call optometrist jokes? Vitreous humor.
Feb 16 2015
next sibling parent "Ulrich =?UTF-8?B?S8O8dHRsZXIi?= <kuettler gmail.com> writes:
On Monday, 16 February 2015 at 14:35:51 UTC, H. S. Teoh wrote:
 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.
So much truth in here. I for one was amazed at how fast the change got in and I am glad it did.
 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
Long mails are time-consuming, too. ;)
 -- I was hoping that, this being a community project, somebody 
 else could have picked up the slack
Hmm, this seems to be happening just now. At least Andrei asks for it to happen.
 without slapping me in the face for
 having not enough time to make everything perfect.
Andrei can be so encouraging. Indeed, the quality he asks for is what makes D so damn attractive.
 In retrospect, perhaps I should never have bothered.
I am glad you did. I try to follow your commits to help me learn D better. For some reason this seems to work for me. Without your work I'd run out of inspiration. So if there is any slack remaining that you cannot attend, please tell me. With some help I might be able to something about it.
Feb 16 2015
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
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
Feb 16 2015
next sibling parent reply "H. S. Teoh via Digitalmars-d" <digitalmars-d puremagic.com> writes:
On Mon, Feb 16, 2015 at 09:36:57AM -0800, Andrei Alexandrescu via Digitalmars-d
wrote:
 On 2/16/15 6:33 AM, H. S. Teoh via Digitalmars-d wrote:
[...]
 One thought about "forward" and "SortOutput" - first may e.g. go in
 std.functional and the latter belongs to "sorting".
https://github.com/D-Programming-Language/phobos/pull/2996
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.
The point is that I *did* take the pains to make sure those links still worked -- otherwise they would look all nice and pretty, exactly the same as before, except that they wouldn't work anymore. The only fault was that I didn't notice any font changes -- I usually don't pay attention to those things, and my browser uses custom fonts anyway so my default assumption is that it's a quirk of my browser, rather than anything wrong with the actual font. [...]
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.
Without a contribution guide, nobody can remember all these scattered things that must be reviewed prior to merging, like making sure dlang.org links aren't broken, making sure the navbar definitions in a separate repo still match up with the current state of Phobos, etc.. [...]
 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.
I disagree. Many users were complaining that importing std.algorithm will pull in tons of Phobos baggage due to the mass of import dependencies. Ilya, IIRC, wasted days trying to narrow down a compiler bug because std.algorithm dependency soup made it very difficult to reduce the problem past a certain point inside Phobos. A lot of preliminary work was actually done by him -- localizing imports, reducing inter-module dependencies, etc.. The new std.algorithm submodules, while far from perfect I'll admit, at least limit the scope of transitive dependencies in user code. This improves compile times, and makes Phobos more modular for people who care about not pulling everything including the kitchen sink into their executables.
 * Even the value to contributors is debatable and definitely low impact.
I completely disagree. You are obviously unaware of the fact that the reason I embarked on this fool's errand in the first place was because std.algorithm had grown so large and unwieldy that I could not even run unittests on my machine anymore -- the compiler just dies with an out-of-memory fork error. That means (1) I cannot test my changes locally; (2) I have to rely on the autotester, thereby adding unnecessary load for experimental changes that are almost guaranteed to fail. This increases the barrier of entry to would-be contributors, since not everybody has the latest and greatest souped-up PC with gobs of spare RAM, and they would also experience (1) and (2). I certainly did not sit down one day with too much free time on my hands and tell myself "hmm, I'm bored today, I wonder which Phobos module I can waste time on by needlessly refactoring it." The reason I even contemplated such a foolhardy idea at all was because it was becoming increasingly difficult for me to do any meaningful work on std.algorithm due to its unwieldy size. I think it's totally unfair to call this "low impact".
 * The documentation is in shambles.
It *wasn't* in shambles before this?
 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.
I must admit I can't wrap my head around equating "make it possible for contributors to run Phobos unittests locally without needing to upgrade their PC" with "net negative" and "low-impact artifact". Am I really that out of touch with reality? [...]
 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.
Again, I'm baffled by your continual insistence on calling something "low impact" when it is affecting how contributors can successfully run Phobos unittests without running out of memory, or reducing the amount of interdependencies between Phobos modules so that it's easier to isolate problems and pull in only stuff that's necessary for a particular piece of code instead of pulling in the entire hairball of dependencies that will end up linking half of Phobos to your program just to call a single function. Obviously, what I care about does not match what you care about, so it would seem that the path of least pain would be for me to take a break from contributing. [...]
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.
[...] It's not about my work being shit or not. If my work is shit I would rather appreciate it if somebody alerted me to that fact, or better yet, improve on it so that it's not shit. It's about spending a ton of my spare time to make something I care about work (i.e., being able to run std.algorithm unittests locally without running out of memory so that I can improve the quality of my PRs), and then being told afterwards that it was essentially pointless churn that adds no value, or worse, a net negative. I don't disagree that it could have been carried out in a better way than it turned out, but being told that making things easier for others was essentially unwanted work really makes me reconsider contributing. I understand that you are pushing to improve the quality of the website, the general quality of PRs, etc., but there's no need to trample on peripheral improvements in the meantime, however marginal they may seem to you. T -- Food and laptops don't mix.
Feb 16 2015
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/16/15 11:46 AM, H. S. Teoh via Digitalmars-d wrote:
 On Mon, Feb 16, 2015 at 09:36:57AM -0800, Andrei Alexandrescu via
Digitalmars-d wrote:
 On 2/16/15 6:33 AM, H. S. Teoh via Digitalmars-d wrote:
[...]
 One thought about "forward" and "SortOutput" - first may e.g. go in
 std.functional and the latter belongs to "sorting".
https://github.com/D-Programming-Language/phobos/pull/2996
Great, thanks. Per the comment in the pull request, if forward had been available in previous releases from std.algorithm, you may want to add a public alias for it. I appreciate you took the time to answer this sagely and to take action. I stand corrected and agree it's important that builds unittests don't run unreasonably large. Let's focus on landing this smoothly and on deriving good learning for the future. Thanks, Andrei
Feb 16 2015
prev sibling parent ketmar <ketmar ketmar.no-ip.org> writes:
On Mon, 16 Feb 2015 09:36:57 -0800, Andrei Alexandrescu wrote:

 * There is no added value to D users in splitting algorithm.d into a
 package.
=20
 * Even the value to contributors is debatable and definitely low impact.
=20
 * The documentation is in shambles.
=20
 All told we're looking at a net negative.
fuckin' perfect. and then you asking why people don't want to contribute=20 and how to convert 'em to contributors. it's like beating a dog and then=20 asking why it doesn't want to come closer anymore, and what was wrong. i bet telling people that their work is "low-impact" and "net negative"=20 is one of the best ways to attract contributors. especially when people=20 were care to explain why exactly that work was done, what it solves and=20 why it's needed. sure, this shouldn't be taken emotionally, 'cause well, why somebody=20 should be emotional about the work he did in his spare time and cared to=20 share with others? he has nothing better to do anyway.=
Feb 16 2015