feat: add country-to-continent mapping for issue labels - #1477
Conversation
Country to Continent mapping found here: https://gist.github.com/stevewithington/20a69c0b6d2ff846ea5d35e5fc47f26c Addressing MobilityData/mobility-database-catalogs#1005
ianktc
changed the title
|
Could probably move these to a constant in another file cause its alot of lines. |
|
*Lighthouse ran on https://mobility-feeds-dev--pr-1477-3qk4cuff.web.app/ * (Desktop)
*Lighthouse ran on https://mobility-feeds-dev--pr-1477-3qk4cuff.web.app/feeds * (Desktop)
*Lighthouse ran on https://mobility-feeds-dev--pr-1477-3qk4cuff.web.app/feeds/gtfs/mdb-2126 * (Desktop)
*Lighthouse ran on https://mobility-feeds-dev--pr-1477-3qk4cuff.web.app/feeds/gtfs_rt/mdb-2585 * (Desktop)
*Lighthouse ran on https://mobility-feeds-dev--pr-1477-3qk4cuff.web.app/gbfs/gbfs-flamingo_porirua * (Desktop)
|
|
Preview Firebase Hosting URL: https://mobility-feeds-dev--pr-1477-3qk4cuff.web.app |
[suggestion] Checkout |
Sorry for so many commits 😅 I found a ts library that seems reputable: https://www.npmjs.com/package/countries-list |
| labels.push(continent); | ||
| } | ||
| } catch (error) { | ||
| logger.error("Error mapping ${formData.country} to continent", error); |
There was a problem hiding this comment.
| logger.error("Error mapping ${formData.country} to continent", error); | |
| logger.error(`Error mapping ${formData.country} to continent`, error); |
need to use ` to display ${formData.country} variable
| try { | ||
| if (formData.country && formData.country in countries) { | ||
| const country = countries[formData.country as TCountryCode]; | ||
| const continent = continents[country.continent].toLowerCase(); | ||
| labels.push(continent); | ||
| } | ||
| } catch (error) { | ||
| logger.error("Error mapping ${formData.country} to continent", error); |
There was a problem hiding this comment.
no need for a try catch, you're better off handling the undefined case explicitly
const continent = continents[country.continent].toLowerCase();
if(continent != null) labels.push(continent)
Alessandro100
left a comment
Alessandro100
left a comment
There was a problem hiding this comment.
lgtm, nice use of the countries-list library
|
🥳 Congrats on getting your first pull request merged! |
3 participants
Country to Continent mapping found here: https://gist.github.com/stevewithington/20a69c0b6d2ff846ea5d35e5fc47f26c
Addressing MobilityData/mobility-database-catalogs#1005
Google Sheet: https://docs.google.com/spreadsheets/d/1mP4330nAewh5X5Z6XeVei6IJ-YudaEC-asdPAixrol4/edit?gid=1816052211#gid=1816052211
Summary:
Summarize the changes in the pull request including how it relates to any issues (include the #number, or link them).
Expected behavior:
Explain and/or show screenshots for how you expect the pull request to work in your testing (in case other devices exhibit different behavior).
Testing tips:
Provide tips, procedures and sample files on how to test the feature.
Testers are invited to follow the tips AND to try anything they deem relevant outside the bounds of the testing tips.
Please make sure these boxes are checked before submitting your pull request - thanks!
./scripts/api-tests.shto make sure you didn't break anything