-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add field for a SHA, with autocomplete #689
Conversation
Staging instance deployed by Travis CI! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM % a nit
let same = true; | ||
for (let i = 0; i < shas.length && same; i++) { | ||
if (i >= curr.length || shas[i] !== curr[i]) { | ||
same = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few suggestions about this loop:
- Why do we need to check
i >= curr.length
? The whole loop only runs ifcurr
andshas
have the same length. - We can simply break here, instead of checking
same
every time. - Once 1 & 2 are done, then we can simplify the 3-clause for into a for...of (which, unlike for...in, is guaranteed to iterate in order).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to match the versions autocomplete logic I simplified earlier today.
Description
Starting towards #687 (for the all-the-same SHA case)
Adds a WPT flag for
queryBuilderSHA
which supports fetching the/api/shas
endpoint for the current query, and populating the autocomplete of the SHA field using possibilities.Review Information
Visit
/flags
, enable bothqueryBuilder
andqueryBuilderSHA
, type stuff.