Bug 27167 – bugzilla-tool: hide help for unsupported commands

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[David Kilzer (:ddkilzer)]

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(-)

[Eric Seidel (no email)]

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)

[David Kilzer (:ddkilzer)]

Created attachment 32599 [details]
Patch v2

[David Kilzer (:ddkilzer)]

(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!

[Eric Seidel (no email)]

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!

[David Kilzer (:ddkilzer)]

(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.  :(

[Eric Seidel (no email)]

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. :)