Created attachment 32596 [details] Patch v1 Reviewed by NOBODY (OOPS!). When bugzilla-tool -h|--help is invoked in an svn working directory, don't print help for commands that are only supported on git working directories. * Scripts/bugzilla-tool: (Command.requires_local_commits): Added. Must be implemented by all subclasses. (BugsInCommitQueue.requires_local_commits): Added. (PatchesInCommitQueue.requires_local_commits): Added. (ReviewedPatchesOnBug.requires_local_commits): Added. (ApplyPatchesFromBug.requires_local_commits): Added. (LandAndUpdateBug.requires_local_commits): Added. (LandPatchesFromBugs.requires_local_commits): Added. (CommitMessageForCurrentDiff.requires_local_commits): Added. (ObsoleteAttachmentsOnBug.requires_local_commits): Added. (PostDiffAsPatchToBug.requires_local_commits): Added. (PostCommitsAsPatchesToBug.execute): Removed SCM.supports_local_commits() check since this is now handled by BugzillaTool.main(). (PostCommitsAsPatchesToBug.requires_local_commits): Added. (BugzillaTool.commands_usage): Don't print help for commands if they require local commits and the current SCM doesn't support them. (BugzillaTool.main): If command_object requires local commits and the current SCM doesn't, exit with an error message. --- 2 files changed, 82 insertions(+), 7 deletions(-)
Comment on attachment 32596 [details] Patch v1 I think this is great, except I would have defaulted "requires_local_commits" to False, and made it a parameter to the Command constructor (__init__), then you only need to change about 4 lines of code, instead of all the ones you did here. I guess I should r- this since I wouldn't recommend committing as-is, but in general I think this feature is great! I would have just used a slightly different python approach. basically now: Command.__init__(self, ..., requires_local_commits=False) and the few commands which do, call: Command.__init__(self, 'r+\'d patches on a bug', 'BUGID', requires_local_commits=True)
Created attachment 32599 [details] Patch v2
(In reply to comment #1) > (From update of attachment 32596 [details]) > I think this is great, except I would have defaulted "requires_local_commits" > to False, and made it a parameter to the Command constructor (__init__), then > you only need to change about 4 lines of code, instead of all the ones you did > here. Yes, that's much nicer. Thanks!
Comment on attachment 32599 [details] Patch v2 Looks great. I would make: if command_object.requires_local_commits and not self.scm().supports_local_commits(): 591 error(command_name + " requires local commits.") more verbose. You could spit out the SCM.display_name() as well as possibly the root directory we detected for the SCM. Thanks!
(In reply to comment #4) > (From update of attachment 32599 [details]) > Looks great. > > I would make: > if command_object.requires_local_commits and not > self.scm().supports_local_commits(): > 591 error(command_name + " requires local commits.") > > more verbose. You could spit out the SCM.display_name() as well as possibly > the root directory we detected for the SCM. Done! $ git svn dcommit Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebKitTools/ChangeLog M WebKitTools/Scripts/bugzilla-tool Committed r45746 http://trac.webkit.org/changeset/45746 Sorry, I need to land my land-commits patch before I can land my own patches. :(
Nothing to be sorry about. ;) I use two git checkouts for just this reason. One to work on bugzilla-tool from, and one to work on everything else from. :)