mach decorator and dispatching cleanup

RESOLVED FIXED in Firefox 41

Status

RESOLVED FIXED
4 years ago
10 months ago

People

(Reporter: gps, Assigned: gps)

Tracking

unspecified
mozilla41

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(4 attachments)

(Assignee)

Description

4 years ago
As part of implementing a new mach feature, I made some cleanup in the decorator and dispatch code. Patches to come shortly...
(Assignee)

Comment 1

4 years ago
Created attachment 8625178 [details]
MozReview Request: Bug 1176620 - Use absolute_import in mach; r?ahal

Bug 1176620 - Use absolute_import in mach; r?ahal

To help ensure Python 3 compatibility.
Attachment #8625178 - Flags: review?(ahalberstadt)
(Assignee)

Comment 2

4 years ago
Created attachment 8625179 [details]
MozReview Request: Bug 1176620 - Refactor how mach command metadata is stored; r?ahal

Bug 1176620 - Refactor how mach command metadata is stored; r?ahal

Up to this point, mach command metadata has been stored in tuples.
Initially, things weren't so bad. But they have evolved into tuples with
many elements. Adding new attributes is cumbersome. Let's restructure
the code to capture metadata in a dedicated class.

Before, there existed a separate attribute on the @Command or
@SubCommand decorated method for each mach decorator: @Command,
@CommandArgument, @CommandArgumentGroup. With the magic of __ior__,
we can now capture all metadata on a single type. This simplies
processing, as we now only look at a single attribute on methods:
_mach_command.

Before, we used separate attributes to distinguish between mach commands
and mach sub-commands. Now that we have a type that can hold all data,
we combine things into the _mach_command attribute and look for the
presence of the "subcommand" attribute on this type to identify
sub-commands.
Attachment #8625179 - Flags: review?(ahalberstadt)
(Assignee)

Comment 3

4 years ago
Created attachment 8625180 [details]
MozReview Request: Bug 1176620 - Pass fewer arguments into MethodHandler; r?ahal

Bug 1176620 - Pass fewer arguments into MethodHandler; r?ahal

Simplify construction of mach's MethodHandler instances by by passing in
our new rich type that holds all command metadata.

While we are here, kill the docstring argument, as it can be computed
easily inside MethodHandler.__init__.
Attachment #8625180 - Flags: review?(ahalberstadt)
(Assignee)

Comment 4

4 years ago
Created attachment 8625181 [details]
MozReview Request: Bug 1176620 - Eliminate MethodHandler; r?ahal

Bug 1176620 - Eliminate MethodHandler; r?ahal

This type is now redundant with our new rich type for capturing all mach
command metadata. Eliminate it and using _MachCommand instead.
Attachment #8625181 - Flags: review?(ahalberstadt)
Attachment #8625178 - Flags: review?(ahalberstadt) → review+
Comment on attachment 8625178 [details]
MozReview Request: Bug 1176620 - Use absolute_import in mach; r?ahal

https://reviewboard.mozilla.org/r/11799/#review10317

I read somewhere that cwd is on sys.path by default, so "." would still be used as a root for searching absolute paths. Not sure if this is still the case, but just fyi. r+ either way.
Comment on attachment 8625179 [details]
MozReview Request: Bug 1176620 - Refactor how mach command metadata is stored; r?ahal

https://reviewboard.mozilla.org/r/11801/#review10347

Ship It!
Attachment #8625179 - Flags: review?(ahalberstadt) → review+
Comment on attachment 8625180 [details]
MozReview Request: Bug 1176620 - Pass fewer arguments into MethodHandler; r?ahal

https://reviewboard.mozilla.org/r/11803/#review10349

Ship It!
Attachment #8625180 - Flags: review?(ahalberstadt) → review+
Comment on attachment 8625181 [details]
MozReview Request: Bug 1176620 - Eliminate MethodHandler; r?ahal

https://reviewboard.mozilla.org/r/11805/#review10351

Ship It!
Attachment #8625181 - Flags: review?(ahalberstadt) → review+
(Assignee)

Comment 9

4 years ago
https://reviewboard.mozilla.org/r/11799/#review10353

Yeah, that's probably correct. I don't think it's important though. I'm adding absolute_import to everything as a matter of principle: not trying to incur more technical debt for Python 3.

Updated

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