Status

RESOLVED FIXED
4 years ago
8 months ago

People

(Reporter: gps, Assigned: gps)

Tracking

(Blocks: 1 bug)

unspecified
mozilla37

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

Description

4 years ago
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.
(Assignee)

Comment 1

4 years ago
Created attachment 8532947 [details]
MozReview Request: bz://1108399/gps
Attachment #8532947 - Flags: review?(ali)
Attachment #8532947 - Flags: review?(ahalberstadt)
(Assignee)

Comment 2

4 years ago
/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 :)
(Assignee)

Updated

4 years ago
Attachment #8532947 - Flags: review?(ali)
(Assignee)

Comment 4

4 years ago
/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
(Assignee)

Comment 5

4 years ago
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+
(Assignee)

Comment 8

4 years ago
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.
Attachment #8532947 - Flags: review+
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.
(Assignee)

Updated

4 years ago
Attachment #8532947 - Flags: review?(ahalberstadt)
Attachment #8532947 - Flags: review+
Attachment #8532947 - Flags: feedback+
(Assignee)

Comment 10

4 years ago
/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
(Assignee)

Comment 12

4 years ago
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+
(Assignee)

Comment 14

3 years ago
Comment on attachment 8532947 [details]
MozReview Request: bz://1108399/gps
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+
(Assignee)

Comment 15

3 years ago
Created attachment 8618836 [details]
MozReview Request: Bug 1108399 - Split command group populating into own function
(Assignee)

Comment 16

3 years ago
Created attachment 8618837 [details]
MozReview Request: Bug 1108399 - Handle help on mach sub-commands
(Assignee)

Comment 17

3 years ago
Created attachment 8618838 [details]
MozReview Request: Bug 1108399 - Implement mach sub-command dispatching
(Assignee)

Comment 18

3 years ago
Created attachment 8618839 [details]
MozReview Request: Bug 1108399 - Move mach docs into sphinx
(Assignee)

Comment 19

3 years ago
Created attachment 8618840 [details]
MozReview Request: Bug 1108399 - Improve code linking in mach docs
(Assignee)

Comment 20

3 years ago
Created attachment 8618841 [details]
MozReview Request: Bug 1108399 - Split mach documentation into multiple articles
(Assignee)

Comment 21

3 years ago
Created attachment 8618842 [details]
MozReview Request: Bug 1108399 - Properly link to Python docs
(Assignee)

Comment 22

3 years ago
Created attachment 8618843 [details]
MozReview Request: Bug 1108399 - Fix docs for mach decorators
(Assignee)

Comment 23

3 years ago
Created attachment 8618844 [details]
MozReview Request: Bug 1108399 - Remove outdated comment about hooking up conditional commands
(Assignee)

Comment 24

3 years ago
Created attachment 8618845 [details]
MozReview Request: Bug 1108399 - Properly name command help function
(Assignee)

Comment 25

3 years ago
Created attachment 8618846 [details]
MozReview Request: Bug 1108399 - Introduce decorator for mach sub-commands

Updated

8 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.