1259620 - Add an optional callback to @checking

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[Mike Hommey [:glandium]]

Attachment: MozReview Request: Bug 1259620 - Add an optional formatting callback to @checking. r?gps

Review commit: https://reviewboard.mozilla.org/r/42359/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42359/

Comment 2

[Mike Hommey [:glandium]]

Attachment: MozReview Request: Bug 1259620 - Add @checking to host and target to display the triplets. r?gps

Review commit: https://reviewboard.mozilla.org/r/42361/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42361/

Comment 3

[Gregory Szorc [:gps]]

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.

Comment 4

[Gregory Szorc [:gps]]

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

Comment 5

[Mike Hommey [:glandium]]

(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.

Comment 6

[Gregory Szorc [:gps]]

Fine, land it as is.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/67e1d5126c9f
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7d69f146a52

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/67e1d5126c9f
https://hg.mozilla.org/mozilla-central/rev/a7d69f146a52