Code review comment for lp:~gnuoy/charm-helpers/add-vcsdirs-module

This looks awesome, Liam. Thanks! As this is a contrib feature, I'm game for merging it as-is, but I see a few things that are worth discussing first.

In _get_vcs_identifier, I think it would be best to catch exceptions a bit more specifically. If the user doesn't have the git module, for example, they might get back a generic "Exception" with no description of what went wrong. It would be helpful to provide a description when you throw the exception (e.g. Exception("Something bad happened")). Also, if the srcdir is neither a git nor bzr repo, the function simply returns None. That looks problematic in the places it's used.

In VcsDirs.deploy_dir() (line 174), I see:

   if check_newer and current_branchid >= int(candidate_branchid):

I know git isn't your focus, but I'm pretty sure that the branchids won't be comparable as ints. If fixing that isn't trivial, I certainly wouldn't begrudge you to remove git support for this merge. Also (trivial), I believe the "check_newer" boolean in this line is redundant.

As for the body of this conditional, did you intend to do anything besides report? I would have expected it to do the deploy only if the new code is newer.

Finally, I see that you move the code from its original place rather than copying (or preferably rsync'ing) it. I wonder if that might be somewhat surprising to users. I think it bears mentioning in the docs at least.