Code review comment for lp:~a-s-usov/bzr-fastimport/renaming-tags

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Thanks for the patch. This mostly looks ok; some minor issues:

* two empty lines are necessary before "+class CheckRefnameRewriting(tests.TestCase):" and "+def sanitize_ref_name_for_git(name_dict, refname):", consistent with PEP8.
* The --rewrite-git-tags option needs a test
* Please use underscores rather than camelcasing
* rewrite_dict could be a local variable rather than a class variable

I'm not sure if it's all that useful to try to prevent duplicate tag names, as this is pretty much impossible. Even with your changes there are risks of duplicate tags (e.g. with incremental imports). Likewise, your change may cause tag names to change if another tag is removed.

« Back to merge proposal