Review Alexandru 17-10-2007 --------------------------- [x] The docs folder contains many text files - are they to become pages in the online documentation? I think the place to put this temporary text files is design or even the root of Webdav. (the root is the place to put TODO anyway) - The option class ezcWebdavMemoryBackendOptions contain constants defined which are used in ezcWebdavMemoryBackend (and maybe other classes). Is this the correct way to do things? I think the constants should be defined in ezcWebdavMemoryBackend. # TS: I don't think this is important enough to change it. In fact, the # constants are used in the options so why declare them in the class itself? [x] There is no tutorial. - ezcWebdavMemoryBackend, ezcWebdavSimpleBackend and maybe other classes have the extends and implements on many lines - I thought it was already discussed on IRC to have everything on one line. # TS: Did not notice that. Please clearify and add it to the guidelines so we # can make it consistent. - Sometimes constructors are documented with '@return void'. I thought we don't write the '@return void' anymore (for constructors much less). # TS: Yes, but we did not consider this as an important fix to remove it. - Some methods don't have code blocks. [x] Some classes have the description 'Description missing'. - Instead of array_key_exists( $propertyName, $this->properties ) it can be isset( $this->properties[$propertyName] ) (in __isset() methods) for speed. # TS: No, it cannot. We must use array_key_exists() since we want to work # around the problem that isset() returns false for properties that do exist # but are null. - Almost all the components have this order in the docblocks for methods: - short description - detailed description, examples - @throws - @param - @return Why in Webdav the order is different? # TS: This does not really matter. However, I'll try to pay attention to it # and fix it if i see it during development. [x] In the structs folder the class variables are defined after the constructor. [x] In the requests folder the @copyright and @license is used 2 times. Review Alexandru 31-07-2008 --------------------------- - Some typos in the code and comments: [x] /src/properties/resourcetype.php TYPE_RESSOURCE -> TYPE_RESOURCE # TS: Kept the old constant for BC reasons. [x] /src/transport.php retreiveBody -> retrieveBody # TS: Kept old method for BC reasons, calling the new method only. /src/path_factories/basic.php collectionPathes -> collectionPaths /src/path_factories/automatic.php collectionPathes -> collectionPaths [x] + collectionPathes is not defined as a class variable # TS: I think we should keep these 2 as they are, since emulating the BC using # interceptors would lead to reference issues. /src/structs/collection.php childs -> children # TS: Same here. several files: [x] pathes -> paths !!! [x] ressource -> resource [x] instanciate -> instantiate [x] powerfull -> powerful [x] childs -> children [x] @throws appear multiple times with the same exception (eg. __get) - Some guidelines issues: /src/options/backend_file_options.php useMimeExts -> useMimeExtensions # TS: Changing this would break BC. I don't think we should do it in this # minor case. [x] /src/server.php properties are not documented (even if read-only) constructor is protected, but docblock says private [x] /src/backends/simple.php var_dump leftover [x] /src/structs/display_information*.php class variables appear after the methods definitions