[MERGE] Custom templates in version-info

John Arbash Meinel john at arbash-meinel.com
Tue Nov 13 17:17:32 GMT 2007


John Arbash Meinel has voted tweak.
Status is now: Conditionally approved
Comment:
I think this is better than what we have, so I'm looking forward to it 
landing. But I did have a few more comments.

a) class Template(dict):

In general, I think subclassing dict() is a poor decision. You really 
don't need the full set of dict() operations, so it is better to just 
use a dict, and add a simple add() and maybe remove() (and __getitem__ 
if you prefer) thunks.

b) We generally prefer to test things using unit-tests, rather than 
doctests. Partly because you can be more expressive with unittests, and 
partly because doctests are a bit broken with lazy importing. (this is 
optional)

c) You currently have it so that non-set template parameters are just 
replaced with the empty string. This is okay, though I could also see 
failing with an error, which would key the user in to the fact that they 
may have a typo in their template. On the other hand, maybe they want to 
use a field that wasn't defined in a particular version.
I would probably prefer to err on the side of caution, and give a nice 
user error if the template has a member that isn't available.
At a minimum, we could add that with a '--strict' flag, but it makes 
sense to me to be a default.


For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3C1194696138.5822.2.camel%40laptop%3E



More information about the bazaar mailing list