Closed Bug 1259620 Opened 4 years ago Closed 4 years ago

Add an optional callback to @checking

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

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.
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+
(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)
https://hg.mozilla.org/mozilla-central/rev/67e1d5126c9f
https://hg.mozilla.org/mozilla-central/rev/a7d69f146a52
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.