On Monday, 17 March 2014 at 19:07:44 UTC, Walter Bright wrote:
On 3/17/2014 11:10 AM, Dicebot wrote:
1)
Walter has been pushing for getting this through the review
queue to the point
where I needed to ask Brian to delay voting for his module and
switch to
proceeding with Walter's. It didn't do any harm this time as
Brian got busy
anyway but I am very unhappy that I even had to do it.
Now it suddenly gets cancelled and merged, internal or not
(the very existence
of std.internal rings a bell but it is a different story). Why
bother me and
push on Brian if you are just going to hurry merge it?
The ongoing threads about it made it clear that it was never
going to happen as std.buffer.scopebuffer. I had assumed you
were monitoring that, and I shouldn't have assumed so. I
apologize.
You see, review process exists for the very reason this is not
clear even if it seems so. I will never take the courage of
judging early termination of review simply because it does not
seem to succeed. If anything, I'd try to encourage as much
different input as possible to make decision that is clear to
external observer.
If you want something internal, you just go for it. If you want
something public and reviewed, changing your mind few days after
review request is not something that leaves a good impression.
Consider how an outsider that does not read NG daily may see it.
We always could have added something needed immediately as
internal module _AND_ proceeded with review of possible higher
level alternative than can fit public Phobos.
The objective technical issues raised were all addressed, and
the immutable/const one was corrected and unittests added for
it before it was pulled.
Ok, I have probably not noticed that between the noise which
leads us to...
Some of the issues were subjective, where there are no clearly
right or wrong answers, and a decision needs to be made at some
point.
ScopeBuffer has been there and commented on for about 2 months
now. At last count it had over 4 comments per line of code. It
is probably the most reviewed PR ever.
..it is not something to be proud of. It has got that many
comments because you started to argue against style comments and
queries for some performance data. That is completely out of the
line of normal PR review. It does not matter if you judgement is
right or wrong here - such approach simply creates too much noise
over things that are not truly important.
This is also the reason why high-level review happens before
creating the pull - hard to stay focused otherwise.
It is necessary to resolve a serious issue we have with Phobos
that comes up constantly in public discussions about D. At some
point we've got to move on and resolve this.
I was among those who has been continuously asking for cleaning
Phobos allocations and I feel that this modules about zero of
issues I see. API allocations are much more important to fix than
internal allocations and you still have not answered why you
consider scopebuffer of more priority.
Also while it is important it is not at any hurry and shouldn't
be done hastily simply because it is next discussion topic of the
month.
