Re: Support for OpenDocument packages

Hello Olivier, > > > > So the base class (haven't looked at the code yet) gets methods > > > > for file manipulation added? > > > Not really. It's just loading a file from the Zip/Package, unless > > > it's an Office document, in which case it's back the the previous > > > behaviour. > > I looked at the code and improved your patch a bit; see my cloned > > repository. > I've only seen the change in 2 commits on the master branch. > > Have you also reviewed the 'packages' branch ? It contains much more > changes ;) I had finally the time to review your changes. Here are some comments: - You put several classes in one file, i.e. OpenDocumentPackage_Storage and OpenDocument_Storage. PEAR follows the one-file-per-class rule which e.g. makes autoloading trivial - this rule is broken here - You sometimes do whitespace changes that just break the PEAR Coding Styles and do nothing else. - Apart from that two issues do I like the idea to have base classes that only care about the base file+manifest handling and implement the office document specific things on a layer above them. What I don't like is the class naming; I'll change that. I think it will be along - OpenDocument_Base_Document - OpenDocument_Base_Storage - OpenDocument_Base_Storage_Zip -- Regards/Mit freundlichen Grüßen Christian Weiske -=≡ Geeking around in the name of science since 1982 ≡=-

Attachment: [application/pgp-signature] signature.asc