Duplicate countries to be in lowercase for x-country header update. #1284
+198
−6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Test Seed Generated SuccessfullyTo apply the test seed:
Seed Details
|
goodov
force-pushed
the
lowercase-countries
branch
from
3b150fb to
aed9284
Compare
goodov
force-pushed
the
lowercase-countries
branch
2 times, most recently
from
480bd97 to
f622be2
Compare
goodov
force-pushed
the
lowercase-countries
branch
from
f622be2 to
90e608e
Compare
goodov
added
generate-seed
goodov
force-pushed
the
lowercase-countries
branch
from
90e608e to
951850d
Compare
goodov
force-pushed
the
lowercase-countries
branch
from
951850d to
5eb4851
Compare
atuchin-m
reviewed
Dec 23, 2024
| // check if country in uppercase also exists in lowercase | ||
| const checkCountriesCase = (countries: string[]) => { | ||
| const countrySet = new Set(countries); | ||
| for (const country of countries) { |
There was a problem hiding this comment.
minor: countries.forEach((country) => {...})
There was a problem hiding this comment.
seems like a style preference to me. I prefer the usual for (...) {} syntax similar to what we have in C++ codebase in Chromium.
atuchin-m
reviewed
Dec 23, 2024
|
|
||
| function duplicateCountriesAsLowercase(variationsSeed: VariationsSeed) { | ||
| // For now duplicate the country filters to lowercase. | ||
| const duplicate = (countries: string[]) => { |
There was a problem hiding this comment.
What about Array.from(new Set([...countries, ...countries.map((c) => c.toLowerCase())]))?
There was a problem hiding this comment.
this will change the order of countries
atuchin-m
approved these changes
Dec 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
List countries both in upper and lowercase so we can safely update server to use lowercase value in x-country header.
Related devops issue https://github.com/brave/devops/issues/12890