Re: Review: Support for per-shot looks
Malcolm Humphreys <malcolmh...@...>
I really like most of this change, I have a few comments which are mostly cosmetic.toggle quoted messageShow quoted text
'Default' seems a little redundant in this names.
config.getDefaultContext() vs config.getCurrentContext()
config.getDefaultSearchPath() vs config.getSearchPath()
config.setDefaultSearchPath() vs config.setSearchPath()
'Config' also seems redundant in these names.
config.getConfigRootDir() vs config.getWorkingDir()
config.setConfigRootDir() vs config.setWorkingDir()
'context' varies it's position in few of the argument list. It would be nice if this was more consistent. ie. BuildColorSpaceOps(ops, config, context, src, dst) vs getProcessor(src, dst, context)
It feels better to me to change getProcessor to (context, src, dst) but either way would work.
Can we rename GetPreppedSearchPath to something like ResolveSearchPaths?
What would cause the g_fileExistsCache to be flushed? I could see this could be problematic if a client app was running for a while and files started to change on disk.
In the future is the idea to add a section into the yaml which LoadEnvironmentVariables() can use to limit which variables it looks up?
On 24/12/2010, at 9:06 AM, Jeremy Selan wrote: