zorba-coders team mailing list archive
-
zorba-coders team
-
Mailing list archive
-
Message #01665
Re: [Merge] lp:~ceejatec/zorba/feature-module-installation into lp:zorba
>Looks good in general. Only tried compiling so far but didn't install. I would suggest that at
> least somebody with Windows also tries it.
I built and tested (and even debugged) it on Windows, and at least my own test cases worked fine.
> Minor comments:
> - theURIPath (good name?); theXQPath?
It's used for all URIs, not just .xq files, so I think --uri-path / theURIPath is more accurate.
> - setLibPath or setLibPaths in the public api?
IMHO, a "path" is "a list of directories." See $PATH in shell, $CLASSPATH in Java, etc. So I consistently used "path" for the URI path and Library path. I couldn't change setModulePaths(), but actually that's kind of accurate because what that function now does is set both the URI and Library paths simultaneously.
I spelled out that "path" == "list of directories" in comments and the --help output, so hopefully it's not confusing.
> - should we really remove -module-path from zorbacmd but keep it in the public sctx (backwards
> incompatible?)
I don't think so. I left --module-path there because I assumed people were using it. It is sometimes convenient as well. I originally considered marking it as "deprecated" and I'm still not sure that it shouldn't be...
> - same for ZORBA_MODULES_INSTALL_DIR
Hm. The variable is only for CMake. I didn't think that "backwards compatibility" extended all the way to build configuration variables. I don't think it's possible to maintain that variable even as a "convenience wrapper", because it would break the core/non-core split.
> Will you add documentation & change log before merging?
That is now done.
--
https://code.launchpad.net/~ceejatec/zorba/feature-module-installation/+merge/80040
Your team Zorba Coders is subscribed to branch lp:zorba.
References