Spec: Fix minor type issue in consume budget if permitted #152
Conversation
The algorithm takes a long when it should just take an integer (as the sum of many longs may not fit in a long).
|
@dmcardle, could you PTAL? Thanks! |
| @@ -697,8 +697,8 @@ null |timeout|: | |||
| |currentWallTime|. | |||
| 1. [=set/Append=] |report| to the user agent's [=aggregatable report cache=]. | |||
|
|
|||
| To <dfn algorithm>consume budget if permitted</dfn> given a {{long}} |value|, an | |||
| [=origin=] <var ignore=''>origin</var>, a [=context type=] |api| and a | |||
| To <dfn algorithm>consume budget if permitted</dfn> given an integer |value|, | |||
There was a problem hiding this comment.
Maybe also specify that it's the sum of contribution values?
I also just realized that we never actually "call" this algorithm anywhere. Is that intentional?
There was a problem hiding this comment.
Updated the wording slightly, lmk if that's clearer. And we do call it here :)
There was a problem hiding this comment.
Aha! I couldn't find the reference because I was searching for [=consume in spec.bs and found nothing. Instead, we're calling "consuming budget if permitted" here. Despite not being a literal string match, bikeshed correctly links to the definition, so maybe it's doing some fuzzy matching?
There was a problem hiding this comment.
Yep, bikeshed is very cool! It'll match like this for the first or last word in the name (see here). For future reference, you can click on the definition in the rendered file and it shows all of the references.
| @@ -697,8 +697,8 @@ null |timeout|: | |||
| |currentWallTime|. | |||
| 1. [=set/Append=] |report| to the user agent's [=aggregatable report cache=]. | |||
|
|
|||
| To <dfn algorithm>consume budget if permitted</dfn> given a {{long}} |value|, an | |||
| [=origin=] <var ignore=''>origin</var>, a [=context type=] |api| and a | |||
| To <dfn algorithm>consume budget if permitted</dfn> given an integer |value|, | |||
There was a problem hiding this comment.
Aha! I couldn't find the reference because I was searching for [=consume in spec.bs and found nothing. Instead, we're calling "consuming budget if permitted" here. Despite not being a literal string match, bikeshed correctly links to the definition, so maybe it's doing some fuzzy matching?
|
Thanks for the review! |
SHA: 94f3f55 Reason: push, by alexmturner Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
The algorithm takes a long when it should just take an integer (as the sum of many longs may not fit in a long).
Preview | Diff