On 6/15/16 9:34 PM, tsbockman wrote:
> `checkedint` (and @burner's `SafeInt` before it) have been under
> development in the open for over a year now. There have been several
> discussions in the forums, with feedback being actively solicited.
> Significant design changes were made to address various people's needs.
> `SafeInt` was an open pull request for many months with 100+ comments
> accumulating in that time.
>
> Why didn't you make your design requirements known at any earlier point
> in this process? If you are ultimate gate keeper for Phobos (as you seem
> to be), you ought to make your requirements known *before* the
> implementation is finished.

Apologies about that. I've done a bit of spelunking to see what happened. Indeed the first reference to SafeInt is on a forum post on 6/7/2015, followed immediately by https://github.com/dlang/phobos/pull/3389 which entailed a long discussion.

You first posted about checkedint here on 6/30/2015, in a large thread.

At that time, I had the std.allocator review going on (started on 6/11/2015), a newborn baby, and a move across the continent to worry about (which happened at the end of June). It is entirely possible I just missed that discussion, or more likely saw it and had no meaningful input at the time. There has been a gap in forum posts with "checkedint" in the title between 7/3/2015 and 6/7/2016, so it's not like there was a continuing presence I was working hard to ignore. I honestly think there's nothing to be offended over.

This underlies a larger issue. There must be a protocol that guarantees a proposal is brought to consideration to the D leadership. Dicebot is leading such an initiative (which can be seen as a revamping of DIPs) and we hope to get it finalized soon.


Andrei

On Thursday, 16 June 2016 at 02:53:38 UTC, Andrei Alexandrescu wrote:
> On 6/15/16 9:34 PM, tsbockman wrote:
>> Why didn't you make your design requirements known at any earlier point
>> in this process? If you are ultimate gate keeper for Phobos (as you seem
>> to be), you ought to make your requirements known *before* the
>> implementation is finished.
>
> Apologies about that. I've done a bit of spelunking to see what happened. Indeed the first reference to SafeInt is on a forum post on 6/7/2015, followed immediately by https://github.com/dlang/phobos/pull/3389 which entailed a long discussion.
>
> You first posted about checkedint here on 6/30/2015, in a large thread.
>
> At that time, I had the std.allocator review going on (started on 6/11/2015), a newborn baby, and a move across the continent to worry about (which happened at the end of June). It is entirely possible I just missed that discussion, or more likely saw it and had no meaningful input at the time. There has been a gap in forum posts with "checkedint" in the title between 7/3/2015 and 6/7/2016,

Numerous other mentions were made of this project in various contexts on the forums, in GitHub pull requests, and on the bug tracker - including discussions in which you participated. 'posts with "checkedint" in the title' is too narrow of a search filter.

> so it's not like there was a continuing presence I was working hard to ignore. I honestly think there's nothing to be offended over.

Malicious intent is not required to make the act offensive; you're still jumping into a project a year in the making and demanding that I choose between investing an additional six months (wild guess) of my time working on things I don't care about (at best), or canceling the project (which has otherwise received generally positive feedback so far).

I am not too upset mostly because I had a variety of reasons for pursuing this, not all of which depend on getting it into Phobos.

> This underlies a larger issue. There must be a protocol that guarantees a proposal is brought to consideration to the D leadership. Dicebot is leading such an initiative (which can be seen as a revamping of DIPs) and we hope to get it finalized soon.
>
>
> Andrei

That is part of the problem, but this is also a fine example of a broader pattern that I have noticed in D's review process:

Pull requests are routinely reviewed in an upside-down fashion:

1) Formatting
2) Typos
3) Names
4) Tests (and names again)
6) Docs (and names)
8) Design (and more about names)
9) Does this even belong in Phobos?

I don't think people are doing it on purpose - it's just easier to start with the trivial nit-picks, because you don't need a deep understanding of the code and the problem domain (or decision-making authority) to complain about a missing ' ' or something.

But, that doesn't change the fact that the process still feels almost perfectly designed to waste contributors' time.

Unless the PR is a complete mess, (9) and (8) should be debated *first*, before worrying about any of the other stuff. Why waste people's time fixing trivialities, if it's all going to just be deleted or rewritten anyway?

On Thursday, 16 June 2016 at 03:56:02 UTC, tsbockman wrote:
> On Thursday, 16 June 2016 at 02:53:38 UTC, Andrei Alexandrescu wrote:
>> [...]
>
> Numerous other mentions were made of this project in various contexts on the forums, in GitHub pull requests, and on the bug tracker - including discussions in which you participated. 'posts with "checkedint" in the title' is too narrow of a search filter.
>
>> [...]
>
> Malicious intent is not required to make the act offensive; you're still jumping into a project a year in the making and demanding that I choose between investing an additional six months (wild guess) of my time working on things I don't care about (at best), or canceling the project (which has otherwise received generally positive feedback so far).
>
> I am not too upset mostly because I had a variety of reasons for pursuing this, not all of which depend on getting it into Phobos.
>
>> [...]
>
> That is part of the problem, but this is also a fine example of a broader pattern that I have noticed in D's review process:
>
> Pull requests are routinely reviewed in an upside-down fashion:
>
> 1) Formatting
> 2) Typos
> 3) Names
> 4) Tests (and names again)
> 6) Docs (and names)
> 8) Design (and more about names)
> 9) Does this even belong in Phobos?
>
> I don't think people are doing it on purpose - it's just easier to start with the trivial nit-picks, because you don't need a deep understanding of the code and the problem domain (or decision-making authority) to complain about a missing ' ' or something.
>
> But, that doesn't change the fact that the process still feels almost perfectly designed to waste contributors' time.
>
> Unless the PR is a complete mess, (9) and (8) should be debated *first*, before worrying about any of the other stuff. Why waste people's time fixing trivialities, if it's all going to just be deleted or rewritten anyway?



Has this work/design been submitted as a DIP? I cannot find it.

I thought all Phobos additions of any magnitude were required to pass the DIP submission first in order to avoid this sort of situation. If there is a DIP that was accepted then to have it knocked back now would suck.

Without a DIP you have to expect the design could be turned down by any core developer when they first get the opportunity to review it, no matter how long after the work was initiated.

bye,
lobo

On Thursday, 16 June 2016 at 03:56:02 UTC, tsbockman wrote:
>
> That is part of the problem, but this is also a fine example of a broader pattern that I have noticed in D's review process:
>
> Pull requests are routinely reviewed in an upside-down fashion:
>
> 1) Formatting
> 2) Typos
> 3) Names
> 4) Tests (and names again)
> 6) Docs (and names)
> 8) Design (and more about names)
> 9) Does this even belong in Phobos?
>
> I don't think people are doing it on purpose - it's just easier to start with the trivial nit-picks, because you don't need a deep understanding of the code and the problem domain (or decision-making authority) to complain about a missing ' ' or something.
>
> But, that doesn't change the fact that the process still feels almost perfectly designed to waste contributors' time.
>
> Unless the PR is a complete mess, (9) and (8) should be debated *first*, before worrying about any of the other stuff. Why waste people's time fixing trivialities, if it's all going to just be deleted or rewritten anyway?

I think anything sufficiently large is likely to be reviewed in that order. In a lot of cases all the work for 1 - 8 is progressively done while working out 9. Should people not mention the smaller mistakes / disagreements they find along the way until they've reached the end and can provide a final judgement?

On 6/15/16 11:56 PM, tsbockman wrote:
> Numerous other mentions were made of this project in various contexts on
> the forums, in GitHub pull requests, and on the bug tracker - including
> discussions in which you participated. 'posts with "checkedint" in the
> title' is too narrow of a search filter.

I am sure there were, which was especially visible to you because you were following the project. Some examples would be helpful so I can learn from them.

>> so it's not like there was a continuing presence I was working hard to
>> ignore. I honestly think there's nothing to be offended over.
>
> Malicious intent is not required to make the act offensive; you're still
> jumping into a project a year in the making and demanding that I choose
> between investing an additional six months (wild guess) of my time
> working on things I don't care about (at best), or canceling the project
> (which has otherwise received generally positive feedback so far).

Agreed malice is not required. But I'm still having trouble seeing the offense. Annoyance at a negative review, sure, we're all human. But taking offense? The closest anything came to "demanding" anything has been:

> This suggests a much simpler design [...]
> But I suggest you to reconsider.

How could I have phrased my review and follow-up in ways that are not offensive? Should I have just accepted the proposal on grounds that a lot of work has been put into it and the deadline has passed for influencing it? (Non-rhetorical questions.)

> Pull requests are routinely reviewed in an upside-down fashion:
>
> 1) Formatting
> 2) Typos
> 3) Names
> 4) Tests (and names again)
> 6) Docs (and names)
> 8) Design (and more about names)
> 9) Does this even belong in Phobos?
>
> I don't think people are doing it on purpose - it's just easier to start
> with the trivial nit-picks, because you don't need a deep understanding
> of the code and the problem domain (or decision-making authority) to
> complain about a missing ' ' or something.

I can see how that could be happening. Often (and in this case) there are different folks touching on the different points.


Andrei

On 6/16/16 2:55 AM, lobo wrote:
> Without a DIP you have to expect the design could be turned down by any
> core developer when they first get the opportunity to review it, no
> matter how long after the work was initiated.

We're still working on making DIPs a process that requires leadership to intervene, but I'd say this is already true now.

Per the title of this thread: "std.experimental.checkedint is ready for comments!". What kind of comments were expected?


Andrei

On 6/15/16 11:56 PM, tsbockman wrote:
> On Thursday, 16 June 2016 at 02:53:38 UTC, Andrei Alexandrescu wrote:
>> On 6/15/16 9:34 PM, tsbockman wrote:
>>> Why didn't you make your design requirements known at any earlier point
>>> in this process? If you are ultimate gate keeper for Phobos (as you seem
>>> to be), you ought to make your requirements known *before* the
>>> implementation is finished.
>>
>> Apologies about that. I've done a bit of spelunking to see what
>> happened. Indeed the first reference to SafeInt is on a forum post on
>> 6/7/2015, followed immediately by
>> https://github.com/dlang/phobos/pull/3389 which entailed a long
>> discussion.
>>
>> You first posted about checkedint here on 6/30/2015, in a large thread.
>>
>> At that time, I had the std.allocator review going on (started on
>> 6/11/2015), a newborn baby, and a move across the continent to worry
>> about (which happened at the end of June). It is entirely possible I
>> just missed that discussion, or more likely saw it and had no
>> meaningful input at the time. There has been a gap in forum posts with
>> "checkedint" in the title between 7/3/2015 and 6/7/2016,
>
> Numerous other mentions were made of this project in various contexts on
> the forums, in GitHub pull requests, and on the bug tracker - including
> discussions in which you participated. 'posts with "checkedint" in the
> title' is too narrow of a search filter.
>
>> so it's not like there was a continuing presence I was working hard to
>> ignore. I honestly think there's nothing to be offended over.
>
> Malicious intent is not required to make the act offensive; you're still
> jumping into a project a year in the making and demanding that I choose
> between investing an additional six months (wild guess) of my time
> working on things I don't care about (at best), or canceling the project
> (which has otherwise received generally positive feedback so far).
>
> I am not too upset mostly because I had a variety of reasons for
> pursuing this, not all of which depend on getting it into Phobos.
>
>> This underlies a larger issue. There must be a protocol that
>> guarantees a proposal is brought to consideration to the D leadership.
>> Dicebot is leading such an initiative (which can be seen as a
>> revamping of DIPs) and we hope to get it finalized soon.
>>
>>
>> Andrei
>
> That is part of the problem, but this is also a fine example of a
> broader pattern that I have noticed in D's review process:
>
> Pull requests are routinely reviewed in an upside-down fashion:
>
> 1) Formatting
> 2) Typos
> 3) Names
> 4) Tests (and names again)
> 6) Docs (and names)
> 8) Design (and more about names)
> 9) Does this even belong in Phobos?

This is a good point. I personally have avoided doing anything in 1-8 unless I first agree it's a change I think we should accept. Which then results in 0) nobody is looking at this!???

I will occasionally chime in when there is a particular pain point with design philosophy, but the problem is, this gives the impression that I am willing to accept the change after X nitpicks are done. I try not to do this unless there is another reviewer that seems motivated to do a full review who has already posted, and they haven't covered that piece.

On the other hand, peoples minds can change.

e.g:

"The early iterations of `checkedint` worked this way (although I had no plans to support user-defined hooks). I implemented and debugged it, and thought it was about ready to submit many months ago.

Then I actually tried *using* it, and hated it."

And there is a further problem -- Walter and Andrei are gatekeepers, but are stretched incredibly thin. So a reviewer that is keen on a certain addition may put in a lot of time on steps 1-8 assuming that step 9 is a given, and when W/A come around to looking at it, they say no.

How to fix this? I don't know. The only recommendation I have is to do 2 things:

1. Get pre-approval if you have your heart set on Phobos inclusion. Make SURE you understand the expectations from Andrei and Walter.
2. Be willing to put it on code.dlang.org instead, and Phobos as a secondary goal (your approach).

I had a similar experience with dcollections, though I never really intended on it being in Phobos, it was meant for Tango. But that didn't work out either, as the gatekeeper there was working on his own version of containers.

> Unless the PR is a complete mess, (9) and (8) should be debated *first*,
> before worrying about any of the other stuff. Why waste people's time
> fixing trivialities, if it's all going to just be deleted or rewritten
> anyway?

I understand your frustration. All I can say is, open source contributors have to have a thicker skin (and I'm not saying you don't). We are all human and have our faults, and any team in any context can have miscommunication, or misunderstandings. I can assure you it's not the plan to waste people's time or to cause frustration. Even with a mitigating plan in place, these can happen. Have a plan that you can control, with a viable path if the things you can't don't go your way.

-Steve

On Thursday, 16 June 2016 at 07:02:21 UTC, John Colvin wrote:
> I think anything sufficiently large is likely to be reviewed in that order. In a lot of cases all the work for 1 - 8 is progressively done while working out 9. Should people not mention the smaller mistakes / disagreements they find along the way until they've reached the end and can provide a final judgement?

I think that consideration should be given to splitting reviews into two phases by policy:

    1. The big picture: refining the overall design, and debating
       whether the change is worthwhile or not. This ends when the
       change has been formally approved by someone who has the
       authority to do so.

    2. Completing and polishing the implementation, until it is
       actually ready to merge.

Distinguish clearly between these phases, and make it clear to submitters that they are not required or expected to fix/finish all the little stuff until (1) is over, since there's a good chance it will all be irrelevant, anyway.

Obviously there will be some fuzziness as to whether an issue belongs in (1) or (2), but there's lots and lots of stuff that clearly falls into one or the other.

One of the things that such a policy would accomplish is to highlight the essential (but often ignored) question, "Who actually has authority to approve this?"

On Thursday, 16 June 2016 at 12:25:39 UTC, Andrei Alexandrescu wrote:
>> Pull requests are routinely reviewed in an upside-down fashion:
>>
>> 1) Formatting
>> 2) Typos
>> 3) Names
>> 4) Tests (and names again)
>> 6) Docs (and names)
>> 8) Design (and more about names)
>> 9) Does this even belong in Phobos?
>>
>> I don't think people are doing it on purpose - it's just easier to start
>> with the trivial nit-picks, because you don't need a deep understanding
>> of the code and the problem domain (or decision-making authority) to
>> complain about a missing ' ' or something.
>
> I can see how that could be happening. Often (and in this case) there are different folks touching on the different points.

Yes, it's mostly a process issue, not an individual one. See my earlier reply to John Colvin, for a practical suggestion as to how to improve with this.

On Thursday, 16 June 2016 at 06:55:00 UTC, lobo wrote:
> Has this work/design been submitted as a DIP? I cannot find it.
>
> I thought all Phobos additions of any magnitude were required to pass the DIP submission first in order to avoid this sort of situation. If there is a DIP that was accepted then to have it knocked back now would suck.
>
> Without a DIP you have to expect the design could be turned down by any core developer when they first get the opportunity to review it, no matter how long after the work was initiated.
>
> bye,
> lobo

My observation has been that the DIP process is not followed, in practice. The DIP for this project (which I did not start; I merely continued @burner's work) is actually the same one as that of the recently accepted `ndslice`: https://wiki.dlang.org/DIP80