Hi Julien,

We are getting close :-)

Still passing strings as strings rather than const string&, but I can
changed that :-)

I'm inclined to change the split function to directly add the strings
into the associate list rather than put them in a StringList then copy
it as this will be more efficient and require less code. Again this is
small tweak that I can implement.

Have you tested out the feature yet?

Robert.


On 2 June 2016 at 12:45, Julien Valentin <[email protected]> wrote:
> Here's the new submission
> Hope I didn't miss a thing
>
>
> robertosfield wrote:
>> Hi Julien,
>>
>> On 2 June 2016 at 11:46, Julien Valentin <> wrote:
>>
>> > I may not be clear enough:
>> > If you change the inheritance of a class (such Drawable inheriting from 
>> > Node) the base class serializer is already used by other (Node serializer 
>> > is already used in Group etc...) so you don't want to tag the wrapper with 
>> > a version (tagging Node serializer with newversion would make no sense as 
>> > it's already present in previous version).So You need finer version 
>> > tagging control...
>> >
>>
>> Yes, now makes perfect sense.  Given this constraint my suggestion of
>> embedding the supported versions into the wrapper itself isn't
>> workable.
>>
>> You approach looks to be the best way forward.
>>
>> Your changes aren't ready to merge as is yet as the coding style
>> (indents/spacing/placement of {), are at odds with the rest of code
>> around them. If we merge changes that use different coding styles the
>> OSG code base would end up a bit of a mess and less readable and
>> maintainable for it.  Could you adjust your code to fit in better.
>>
>> Key things are:
>>
>> open { on a a separate line to if etc. statement.
>> four spaces for indentation within {} block
>> no spaces between object->method
>> passing objects by const& rather than as a straight objects. i.e.
>> const std::string& rather than std::string.
>>
>> There will be little ones besides these, general rule If the code
>> looks like it's written be different developers then it's a something
>> that grates.
>>
>> Cheers,
>> Robert.
>> _______________________________________________
>> osg-submissions mailing list
>>
>> http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
>>
>>  ------------------
>> Post generated by Mail2Forum
>
>
> ------------------
> Read this topic online here:
> http://forum.openscenegraph.org/viewtopic.php?p=67346#67346
>
>
>
>
> _______________________________________________
> osg-submissions mailing list
> [email protected]
> http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
>
_______________________________________________
osg-submissions mailing list
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org

Reply via email to