Remove wanted_mozconfig_variables

RESOLVED FIXED in Firefox 48

Status

RESOLVED FIXED
2 years ago
6 months ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

(Depends on: 1 bug)

unspecified
mozilla48

Firefox Tracking Flags

(firefox48 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Comment 1

2 years ago
Created attachment 8741226 [details]
MozReview Request: Bug 1264527 - Remove wanted_mozconfig_variables. r?nalexander

While forgetting about it was warned about, having to add every new
environment option to wanted_mozconfig_variables is cumbersome. It turns
out there is a hackish way to make things work without that list, which,
all things considered, is not worse than the hacks around the
wanted_mozconfig_variables function, and are certainly an improvement as
it doesn't require an ever growing list of environment options.

Review commit: https://reviewboard.mozilla.org/r/46311/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/46311/
Attachment #8741226 - Flags: review?(nalexander)
(Assignee)

Comment 2

2 years ago
Comment on attachment 8741226 [details]
MozReview Request: Bug 1264527 - Remove wanted_mozconfig_variables. r?nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46311/diff/1-2/
Comment on attachment 8741226 [details]
MozReview Request: Bug 1264527 - Remove wanted_mozconfig_variables. r?nalexander

https://reviewboard.mozilla.org/r/46311/#review43049

Wow, that is hacky, but I'm fine with this approach while we transition.

::: build/moz.configure/init.configure:272
(Diff revision 2)
>              # contraints that don't really apply to the command-line
>              # emulation that mozconfig provides.
>              helper.add(arg, origin='mozconfig', args=helper._args)
>  
>          def add(key, value):
> -            # See comment above wanted_mozconfig_variables
> +            if key.isupper():

I generally prefer early return rather than indentation.
Attachment #8741226 - Flags: review?(nalexander) → review+
(Assignee)

Comment 4

2 years ago
https://reviewboard.mozilla.org/r/46311/#review43049

> I generally prefer early return rather than indentation.

For short functions with little level of nesting, I generally prefer not doing early returns. That said, early returns would make the remaining_mozconfig_options function a little better. So I'll do that there.

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/225e6dc0f971
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48

Updated

2 years ago
Depends on: 1288587

Updated

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