[Context: the tail of an exchange between Adam Ruppe and myself. You may want to chime in with thoughts regarding admission of code into Phobos.]

On 04/01/2010 09:33 AM, Adam Ruppe wrote:
> Yes, that'd be very cool!

Sorry for being slow on this. Great! Please send me your username and I'll add it to Phobos.

> Do you guys have a guide as to what goes in and what doesn't? I lot of the stuff I write is "hey, look what I hacked up over the weekend. It works for me!" - I assume phobos should be held to a higher standard than that, but where is the line?

We don't have a formal set of guidelines, and unfortunately there is no mechanism in place to do code reviews before they are checked in. (See my other email.)

There is some code in Phobos that I'm not very fond of (streams) and some code that is well-intended but just "not there" (std.encoding). There's some good code that doesn't conform almost at all with the prevalent Phobos style (std.json).

> There's two levels to this question too: what kind of functionality is in and what is out, and also what kind of sloppiness is out? For example, this code is only set for Linux - before committing, should I test on Windows, or is a "this is linux only for now" function ok too?

I think it's ok to implement something only on one OS initially. It has happened before, and it's actually beneficial because people get encouraged to submit their own port.


Andrei

I have two comments to this.


1. Style guide:

There really should be a formal set of guidelines.  Currently, as a new contributor to the project, I have to look at the various modules, try to guess which ones are up-to-date, and then mimic the style of those modules as best I can.

It seems that someone (Jesse Phillips?) has done quite some work on this page, which looks to me like a good starting point:

   http://prowiki.org/wiki4d/wiki.cgi?DProgrammingGuidelines

Some notable things which are missing from this page:

   - Capitalisation of enums and immutables

   - Free functions vs. static member functions:
         auto r = acquireResource();
         auto r = Resource.acquire();

   - Function template parameters vs. ordinary function parameters:
         enum Option { foo, ... }
         doStuff(someData, Option.foo);
         doStuff!(Option.foo)(someData);

   - Templates vs. new symbols:
         enum Option { bar, baz }
         struct Foo(Option opt) { ... }

         struct FooBar { ... }
         struct FooBaz { ... }


2. Code review:

It seems to me that the sensible thing to do when making major changes to a part of the library is to create a new branch.  (Steve did this for the std.process development.)  Before merging the branch back into trunk, one can send an e-mail to this list requesting feedback and approval.

What is not clear to me, is where to draw the line between a major change that needs approval by the project leader/group, and a minor change that doesn't.

-Lars



Andrei Alexandrescu wrote:
> [Context: the tail of an exchange between Adam Ruppe and myself. You may want to chime in with thoughts regarding admission of code into Phobos.]
> 
> On 04/01/2010 09:33 AM, Adam Ruppe wrote:
>> Yes, that'd be very cool!
> 
> Sorry for being slow on this. Great! Please send me your username and I'll add it to Phobos.
> 
>> Do you guys have a guide as to what goes in and what doesn't? I lot of the stuff I write is "hey, look what I hacked up over the weekend. It works for me!" - I assume phobos should be held to a higher standard than that, but where is the line?
> 
> We don't have a formal set of guidelines, and unfortunately there is no mechanism in place to do code reviews before they are checked in. (See my other email.)
> 
> There is some code in Phobos that I'm not very fond of (streams) and some code that is well-intended but just "not there" (std.encoding). There's some good code that doesn't conform almost at all with the prevalent Phobos style (std.json).
> 
>> There's two levels to this question too: what kind of functionality is in and what is out, and also what kind of sloppiness is out? For example, this code is only set for Linux - before committing, should I test on Windows, or is a "this is linux only for now" function ok too?
> 
> I think it's ok to implement something only on one OS initially. It has happened before, and it's actually beneficial because people get encouraged to submit their own port.
> 
> 
> Andrei
> _______________________________________________
> phobos mailing list
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos