Thanks for catching this! A few comments.
This check will fail for such versions as 2.0.0 or 1.6.3. We'd need to be more specific in our checks. Since it's going to be a bit longer, I'd recommend creating a utility function that checks if a version is at least the version passed. For example: def check_version(version, expected): return (expected > version) or \ (expected == version and expected > version) or \ (expected == version and expected == version and \ expected >= version)) version and expected should be tuples of (major, minor, micro). It would be good to document that in the function. Then your check would become: if not check_version((version_parts.group(1), version_parts.group(2), version_parts.group(2)), (1, 5, 4)): die(...)
I'd rather not have the "post-review failed: " prefix here. If we want a prefix to identify failure, we should put it in die(). Maybe just prefix "fatal: "
Make post-review warn if installed version of git-svn is too old
Review Request #343 — Created April 4, 2008 and submitted
|Review Board SVN (deprecated)|
I installed git-svn the other day with an out-of-date MacPorts index, and wound up with version 126.96.36.199 installed. It turns out that 'git svn info', which is used by post-review to find repository information, wasn't added until git 1.5.4 (see http://www.kernel.org/pub/software/scm/git/docs/RelNotes-1.5.4.txt ). This adds a version check for git-svn which is run if git svn info fails, and which lets the user know that the git-svn version is out of date.
Tested against my old 188.8.131.52 install and my new, hotter 184.108.40.206 install.
Looks good. Committed as r1299. Thanks!