Add an optional callback to @checking

RESOLVED FIXED in Firefox 48

Status

defect
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: glandium, Assigned: glandium)

Tracking

(Blocks 1 bug)

unspecified
mozilla48
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(2 attachments)

Assignee

Description

3 years ago
No description provided.
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 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

3 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)
Fine, land it as is.
Flags: needinfo?(gps)

Comment 8

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/67e1d5126c9f
https://hg.mozilla.org/mozilla-central/rev/a7d69f146a52
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48

Updated

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