Re: Review: YAML serialization
Malcolm Humphreys <malcolmh...@...>
OCIOYaml.cppI think you just fixed this, but yeah there was a bit of code dup.
* You mention in the code, "GroupTransform seems to be a mixedWith the current structure it would be possible to have GroupTransform()s containing GroupTransform()s and possibly more GroupTransform()s and so on..
On a colorspace you can set ColorSpace::setTransform(GroupTransform, dir), to me this seems ok and I do serialize it in this case, but if I support the GroupTransform where I have support for all the other Transform()s then you could build multiple layer deep GroupTransform()s which doesn't seem necessary.
I would prefer to create a new type TransformList eg. ColorSpace::setTransform(TransformList, dir) which would replace GroupTransform and it would different than the base Transform()
Did you try and copy the ExternalProject.cmake into share/cmake to see if that works. I have attached it, can you check to see if it imports ok as I'm not running 2.6.
I have been using 2.8 for about 6 months with no issues, so I would say it's safe to update esp if it helps to reduce the code base.
* On the Imageworks linux system, there is a build error related to libyaml-cpp:I think I was getting a similar error when putting yaml-cpp into the OpenColorIO namespace, I was leaving this to fix later. Did you find a fix for this?
/usr/bin/ld: ../../ext/dist/lib/libyaml-cpp.a(node.cpp.o): relocationUmm I would prefer if this was a patch for the reasons you have stated that it isn't going to change all that much so having it all checked in seems like a waste, while also making it harder to absorb any changes when yaml-cpp changes. The dev on yaml-cpp is pretty active (http://code.google.com/p/yaml-cpp/updates/list)
If we were choosing to fork yaml-cpp (which I don't think we are) or other ext dependants, checking them in would be fine and would help to indicate a fork.
I was also hoping to look at using http://gcc.gnu.org/wiki/Visibility as well to help reduce symbol clashes, have you had any luck/experience with this?
Of course, I'm open to your approach of ExternalProject_Add assuming+1
Let me know if the ExternalProject.cmake works in 2.6, I'm happy to clean up the other externals the same way while I'm still motivated to do so. :)