Formal Review of std.serialization

Jacob Carlborg doob at me.com
Mon Jun 10 23:34:43 PDT 2013


On 2013-06-10 23:40, Jonas Drewsen wrote:
>
> A quick first look for now:
>
> In general I think that you should clone phobos and merge orange into
> std.serialize in order for us to see how it really fits into phobos.
>
> As such I think it feels more like a RFC than formal review because it
> couldn't possible go into phobos in its current state even if we ignored
> all comments from the this list.

Ok. What's already clear:

* orange.serialization.Events -> std.serialization.events
* orange.serialization.RegisterWrapper -> std.serialization.registerwrapper
* orange.serialization.Serializable -> std.serialization.serializable
* orange.serialization.SerializationException -> 
std.serialization.serializationexception
* orange.serialization.Serializer -> std.serialization.serializer
* orange.serialization.archives.Archive -> 
std.serialization.archives.archive
* orange.serialization.archives.XmlArchive -> 
orange.serialization.archives.xmlarchive
* orange.xml.PhobosXml -> (merge with) std.xml

> Now for something more constructive :)
>
>
>> * I'm using some utility functions located in the "util" and "core"
>> packages, what should we do about those, where to put them?
>
> The core package only have the core.Attribute module which defines a
> meta attribute which is a new concept in phobos. I guess that should
> either be removed or we should spawn a discussion about if we want such
> a thing or not. Is it possible to do without it?

Yes, it's possible to do without. But I rather like to start a 
discussion. I'll create a new thread for this.

> The util package
> ----------------
>
> util.use.d : I think the Use struct feels a bit like abusing the 'in'
> operator to work around D not supporting blocks or something else that
> would provide the desired syntax. Personally I think it should go.

Yes, it is. I can move util.Use.restore into XmlArchive and change it to 
use a template delegate instead of using "in".

> util.traits.d : Most of them should go to std.traits or use something
> from std.traits instead if possible

Yes. Some can probably removed.

> util.reflection.d : Should probably be part if std.traits as well since
> there are already some reflection code in there. I guess std.traits
> should make use of the new package.d thing to split up the module into
> several files.
>
> util.ctfe.d : Wouldn't now where to put it.

I could either move that inline where it's used or remove it.

> util.collection.array : should probably go into std.exception

Yes.

> It seems all the module filenames are camel cased. They should be all
> lowercase.

Yes, see above.

> The _.d modules should probably be renamed to package.d now.

Yes, that was like three commits ago :)

> IMHO I think the tests would fit nicely into the package itself. Close
> to the code it tests.

Do you mean in package.d? Yes, if that's possible.

> Could you provide the docs formatted as it would be in dlang.org since
> only that way it is also possible to review formatting.

Ok, I'll see if I can figure it out.

> Keep up the good work. I for one are really looking forward to finally
> getting this thing in.

Cool. Thanks for the review.

-- 
/Jacob Carlborg


More information about the Digitalmars-d mailing list