← Back to team overview

zorba-coders team mailing list archive

Re: [Merge] lp:~zorba-coders/zorba/module-schema-tools into lp:zorba

 

Review: Needs Fixing

Agreed with Matthias's comments.

Regarding DECLARE_ZORBA_JAR(), it looks like the commented-out macro is an older version, and the newer version is not commented-out. The older one should just be deleted.

Couple more comments:

- Do we also need to mention the addition of swig/various.i in NOTICE.xml? It has a comment saying it's from the SWIG package directly.

- Wasn't the class name "IStream" causing a build conflict on Windows?

- The call to init_val() in zorbacmdproperties_base.h should be on a separate line. (I know it was cut-and-pasted from above, but ugh.)

- Why is the method for retrieving the PropertiesGlobal* called "getProperties()"? Shouldn't that be getProperiesGlobal()? (Or, IMHO, getGlobalProperties() and rename the class to GlobalProperties.)

- Similarly, why is the factory method for PropertiesGlobal "Properties::instance()"? (The Zorba Properties stuff has always seemed like a mishmash to me; this isn't helping.)

- Need to fix the conflict in ExternalModules.conf.

- Typo: "singelton" in zorba.h.
-- 
https://code.launchpad.net/~zorba-coders/zorba/module-schema-tools/+merge/97105
Your team Zorba Coders is subscribed to branch lp:zorba.


References