[MERGE] Deprecate export-related InventoryEntry methods
andrew at canonical.com
Mon Jul 28 03:58:50 BST 2008
Andrew Bennetts has voted tweak.
Status is now: Conditionally approved
I'm ok with this, I think.
It seems like a small shame to trade several small methods a function
with large switch statement, though. Perhaps we should refactor again
so that we have something like:
def _dir_export_directory(fullpath, tree):
def _dir_export_file(fullpath, tree):
That way we could fairly easily have unittests that call
dir_exporters['file'] etc directly. At the moment we can't unittest
these pieces easily, because the only way we can invoke this logic is
from the top-level tar/tgz/dir_exporter functions. For instance with
your refactoring a test that exporting to a dir will set or not set the
execute bit as appropriate would have to do a disproportionate amount of
It's possible that a registry like that might be useful to plugins if a
plugin every wants to add a new kind of inventory entry, but that's
probably only a hypothetical situation.
I don't want to get in the way of landing improvements, so I'm voting
"tweak": feel free to merge after considering my comments about tests.
If you feel inspired to add a test_export module, that would be perfect!
For details, see:
More information about the bazaar