[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