Status

enhancement
RESOLVED FIXED
5 years ago
Last year

People

(Reporter: gps, Assigned: gps)

Tracking

(Blocks 1 bug)

unspecified
mozilla37
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(11 attachments, 1 obsolete attachment)

39 bytes, text/x-review-board-request
gps
: review+
Details
39 bytes, text/x-review-board-request
gps
: review+
Details
39 bytes, text/x-review-board-request
gps
: review+
Details
39 bytes, text/x-review-board-request
gps
: review+
Details
39 bytes, text/x-review-board-request
gps
: review+
Details
39 bytes, text/x-review-board-request
gps
: review+
Details
39 bytes, text/x-review-board-request
gps
: review+
Details
39 bytes, text/x-review-board-request
gps
: review+
Details
39 bytes, text/x-review-board-request
gps
: review+
Details
39 bytes, text/x-review-board-request
gps
: review+
Details
39 bytes, text/x-review-board-request
gps
: review+
Details
A few times I've wanted to write sub commands in mach. e.g. |mach command foo| and |mach command bar|. The work I'm doing in bug 1108293 is the latest such driver.

Let's implement sub commands in mach.
Posted file MozReview Request: bz://1108399/gps (obsolete) —
Attachment #8532947 - Flags: review?(ali)
Attachment #8532947 - Flags: review?(ahalberstadt)
/r/1233 - Bug 1108399 - Move mach docs into sphinx
/r/1235 - Bug 1108399 - Improve code linking in mach docs
/r/1237 - Bug 1108399 - Split mach documentation into multiple articles
/r/1239 - Bug 1108399 - Properly link to Python docs
/r/1241 - Bug 1108399 - Fix docs for mach decorators
/r/1243 - Bug 1108399 - Remove outdated comment about hooking up conditional commands
/r/1245 - Bug 1108399 - Properly name command help function
/r/1247 - Bug 1108399 - Introduce decorator for mach sub-commands
/r/1249 - Bug 1108399 - Split command group populating into own function
/r/1251 - Bug 1108399 - Handle help on mach sub-commands
/r/1253 - Bug 1108399 - Implement mach sub-command dispatching

Pull down these commits:

hg pull review -r fcebbe1c01c96327c5fe7c2866bf8a231e51f24e
FYI: You've flagged wrong Ali to review I believe :)
Attachment #8532947 - Flags: review?(ali)
/r/1233 - Bug 1108399 - Move mach docs into sphinx
/r/1235 - Bug 1108399 - Improve code linking in mach docs
/r/1237 - Bug 1108399 - Split mach documentation into multiple articles
/r/1239 - Bug 1108399 - Properly link to Python docs
/r/1241 - Bug 1108399 - Fix docs for mach decorators
/r/1243 - Bug 1108399 - Remove outdated comment about hooking up conditional commands
/r/1245 - Bug 1108399 - Properly name command help function
/r/1247 - Bug 1108399 - Introduce decorator for mach sub-commands
/r/1249 - Bug 1108399 - Split command group populating into own function
/r/1251 - Bug 1108399 - Handle help on mach sub-commands
/r/1253 - Bug 1108399 - Implement mach sub-command dispatching

Pull down these commits:

hg pull review -r fcebbe1c01c96327c5fe7c2866bf8a231e51f24e
Ali: Oops, sorry about that.
https://reviewboard.mozilla.org/r/1231/#review725

::: python/mach/mach/decorators.py
(Diff revision 1)
> +    # reconcile state after traversal.

Typo: paren't. Also either 'hold' or 'reconcile' should be removed.

::: python/mach/mach/decorators.py
(Diff revision 1)
> +    def __init__(self, command, subcommand, description=None):

Shouldn't this also accept the parser argument?

::: python/mach/mach/dispatcher.py
(Diff revision 1)
> +            if not args:

Does this mean if |mach command subcommand| exists it's not possible to run |mach command| on its own? Seems like that could sometimes be desirable.
Comment on attachment 8532947 [details]
MozReview Request: bz://1108399/gps

It's not really clear if I should be flipping the bz flag after a mozreview. I'd give an r+ with my questions answered.
Attachment #8532947 - Flags: review?(ahalberstadt) → feedback+
https://reviewboard.mozilla.org/r/1231/#review941

> Shouldn't this also accept the parser argument?

No. @CommandArgument works for both @Command and @SubCommand.

> Does this mean if |mach command subcommand| exists it's not possible to run |mach command| on its own? Seems like that could sometimes be desirable.

For now, yes. I would be in favor of adding this feature later, if it is ever needed.
https://reviewboard.mozilla.org/r/1231/#review945

Feel free to ignore the parser argument if it doesn't make sense to do that, I don't want to hold this up over that.
Attachment #8532947 - Flags: review?(ahalberstadt)
Attachment #8532947 - Flags: review+
Attachment #8532947 - Flags: feedback+
/r/1233 - Bug 1108399 - Move mach docs into sphinx
/r/1235 - Bug 1108399 - Improve code linking in mach docs
/r/1237 - Bug 1108399 - Split mach documentation into multiple articles
/r/1239 - Bug 1108399 - Properly link to Python docs
/r/1241 - Bug 1108399 - Fix docs for mach decorators
/r/1243 - Bug 1108399 - Remove outdated comment about hooking up conditional commands
/r/1245 - Bug 1108399 - Properly name command help function
/r/1247 - Bug 1108399 - Introduce decorator for mach sub-commands
/r/1249 - Bug 1108399 - Split command group populating into own function
/r/1251 - Bug 1108399 - Handle help on mach sub-commands
/r/1253 - Bug 1108399 - Implement mach sub-command dispatching

Pull down these commits:

hg pull review -r f81c4c55db4b262e6c3f52d638f66b75d2d5d4e2
Comment on attachment 8532947 [details]
MozReview Request: bz://1108399/gps

Not sure why r+ from MozReview didn't come through :/
Attachment #8532947 - Flags: review?(ahalberstadt) → review+
Attachment #8532947 - Attachment is obsolete: true
Attachment #8618836 - Flags: review+
Attachment #8618837 - Flags: review+
Attachment #8618838 - Flags: review+
Attachment #8618839 - Flags: review+
Attachment #8618840 - Flags: review+
Attachment #8618841 - Flags: review+
Attachment #8618842 - Flags: review+
Attachment #8618843 - Flags: review+
Attachment #8618844 - Flags: review+
Attachment #8618845 - Flags: review+
Attachment #8618846 - Flags: review+
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.