Closed
Bug 1259620
Opened 8 years ago
Closed 8 years ago
Add an optional callback to @checking
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(firefox48 fixed)
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: glandium, Assigned: glandium)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
No description provided.
Assignee | ||
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/42359/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42359/
Attachment #8734559 -
Flags: review?(gps)
Assignee | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/42361/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42361/
Attachment #8734560 -
Flags: review?(gps)
Comment 3•8 years ago
|
||
Comment on attachment 8734559 [details] MozReview Request: Bug 1259620 - Add an optional formatting callback to @checking. r?gps https://reviewboard.mozilla.org/r/42359/#review38931 ::: build/moz.configure/checks.configure:27 (Diff revision 1) > # def check(value): > # ... > +# An optional callback can be given, that will be used to format the returned > +# value when displaying it. > @template > -def checking(what): > +def checking(what, callback=None): `callback` feels a bit generic to me. But I suppose we can mass rename it later. It's not like we have to maintain API compatibility. ::: build/moz.configure/checks.configure:63 (Diff revision 1) > if not (isinstance(progs, tuple) or isinstance(progs, list)): > configure_error('progs should be a list or tuple!') > progs = list(progs) > > @depends(var) > - @checking('for %s' % var.lower()) > + @checking('for %s' % var.lower(), lambda x: x or 'not found') Please use a named argument.
Attachment #8734559 -
Flags: review?(gps) → review+
Comment 4•8 years ago
|
||
Comment on attachment 8734560 [details] MozReview Request: Bug 1259620 - Add @checking to host and target to display the triplets. r?gps https://reviewboard.mozilla.org/r/42361/#review38933
Attachment #8734560 -
Flags: review?(gps) → review+
Assignee | ||
Comment 5•8 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #3) > Please use a named argument. Why? If we're not going to change the name, callback is kind of redundant with the fact it's a lambda. As for a better name than callback, I don't have an idea that would be short. A long name would also add noise on the call site.
Flags: needinfo?(gps)
https://hg.mozilla.org/integration/mozilla-inbound/rev/67e1d5126c9f https://hg.mozilla.org/integration/mozilla-inbound/rev/a7d69f146a52
Comment 8•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/67e1d5126c9f https://hg.mozilla.org/mozilla-central/rev/a7d69f146a52
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•