Formal Review Process

Jonathan M Davis jmdavisProg at gmx.com
Wed Jun 12 23:23:50 PDT 2013


On Thursday, June 13, 2013 05:42:43 Jesse Phillips wrote:
> On Wednesday, 12 June 2013 at 03:23:54 UTC, Jonathan M Davis
> 
> wrote:
> > If it's not pull-request ready, then it's not showing how it
> > would fit into Phobos.
> 
> I agree.
> 
> > Where do they fit into Phobos?
> 
> This has been what I'm challenging. The problem I'm trying to
> solve, and probably inappropriately doing so as part of the
> formal review, is as below.
> 
> 1. We do not have a formal way to confirm a library should be put
> into Phobos

What's there to confirm? Anyone who wants it to be in Phobos can prepare it for 
review, and if it passes the vote, it's in. If the programmer doing it wants 
to try and figure out ahead of time whether a particular library or piece of 
functionality might be acceptable, they can just ask, and while that may not 
give a definitive answer, it should avoid the cases where it's an obvious no.

> 2. When Phobos does not provide the supporting modules we do not
> have a formal way to answer "Where do they fit in?"

I would expect that the answer to that is generally either:

1. Get the supporting stuff into Phobos first.

or

2. Make it private to what's being submitted for review, and the private 
functionality can be moved into the public API where appropriate later (either 
through pull requests if it's small or other reviews if it's large - though if 
it's particularly large, I would think that it would be best to get it into 
Phobos on its own first).

It's already happened in the past that some functionality has been private to 
a module and not been put into Phobos' public API. That's essentially what 
happened with portions of std.regex which have recently been voted in as part 
of the new std.uni.

> I'm having a hard time concisely expressing myself with this. In
> principle I completely understand why you're taking this
> position, but practically I don't see it.
> 
> Let me go back to real examples:
> https://github.com/jacob-carlborg/orange
> 
> Orange has a directory for 'tests.' Phobos has generally not had
> unittests separate from the function/module being tested. Is that
> unacceptable? I don't know, there certainly was no objection when
> asked.

I believe that I've objected every time that it's been suggested and I was 
involved in the thread, but I don't know what the consensus of the Phobos devs 
would be on that.

> The repository provides a means to build the source into a
> library. Does it really make it harder to review because someone
> can build and use the code without checking out dmd, druntime,
> phobos and building themselves a testable Phobos library?
> 
> There is also a wiki folder, it has an orange ball. Ok, yes there
> are distractions.
> 
> I'll admit, I started from the docs, diving into the code as I
> needed and had I started from the github repository it wouldn't
> have been obvious where things would go in Phobos.

I think that it pretty much comes down to this: Someone who wants to submit 
something to Phobos gets it to the point that it could be merged into Phobos 
as-is if it were voted in. If there are questions, they can ask in the 
newsgroup ahead of time, but in some cases, they're likely just going to have 
to decide themselves on what they think the best way to do it is. If such a 
decision is not acceptable, then it can then be examined as part of the review 
process.

But essentially what it comes down to is that the submitter submits something 
which they think is in a state which could be merged into Phobos, and then 
it's examined during the review process, and the feedback generated during the 
review process leads to whatever adjustments need to be made. And if it still 
isn't acceptable at that point, then presumably, it'll fail the vote (or not 
even get voted on if it's clearly not ready yet).

- Jonathan M Davis


More information about the Digitalmars-d mailing list