Formal review of std.buffer.scopebuffer

[Dicebot]

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.