Description

Comments

Posted by Robin Skoglund (robinsk) on 2009-07-21T04:57:56.000+0000

In my opinion, ZF should stop mixing SCM properties in files. I find it weird, clumsy, and unnecessary that each file in a repository has a line that says when the file was last updated in the particular source control repository the file is tied to at the moment. This information is available in the SCM itself, and should not have to be part of the actual file contents.

Posted by Thomas Weidner (thomas) on 2009-07-21T05:16:41.000+0000

This information is usefull for debugging.
Often a issue is raised against a release or trunk version and when the user gives this revision number we can see if he is at the latest release (for example switched incubator with core directory and such).

Also users often only take parts of the framework itself or even mix components from one version with some from another version (because things are fixed in the other version).

Even if we always say that this should not be done, in fact it is done.
This ID line is the only way for us to know what revision this file is.

Posted by Pádraic Brady (padraic) on 2009-07-21T05:18:06.000+0000

I like having the ID set on normal class files - it's not hugely informative but it can be helpful to have something at a glance before calling up svn log or svn blame for the details. I'm not sure it's necessary for unit tests though - I certainly haven't added them. Of course I don't even add comment headers to unit tests - they show nothing unique or licensable other than use cases for the classes.

I would also advise against adding more overburdening conditions into the coding standard - they are getting so restrictive that I've taken (with a little bit of shame) to ignoring them in favour of the PEAR CS. I noticed for the first time last week the following gem: Use of the "elseif" construct is not allowed in favor of the "else if" combination. It's a nice touch, but it's also the secondary form of that keyword - not the primary. It will also produce a parse error when you try it within a view script!

Posted by Robin Skoglund (robinsk) on 2009-07-21T05:30:53.000+0000

Point taken. I can see now how it might be useful in some situations. It shouldn't be necessary for unit tests, though, as Pádraic states.

I also agree with Pádraic on the 'elseif' / 'else if' struct, but that's another issue :)