-
Notifications
You must be signed in to change notification settings - Fork 192
This branch is to add "The Bahamas" queryPreset in presets.go 🇧🇸 #157
base: master
Are you sure you want to change the base?
This branch is to add "The Bahamas" queryPreset in presets.go 🇧🇸 #157
Conversation
Add list of principal islands in The Bahamas
ashkulz
left a comment
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.
Please use + instead of space between words.
Add '+' rather than space between words -Update country names from lowercase first letters to capital letters
presets.go
Outdated
| "the bahamas": QueryPreset{ | ||
| include: []string{"The+Bahamas", "Abaco", "Acklins", "Andros", "Berry+Islands", "Bimini", "Cat+Island", "Crooked+Island", "Eleuthera", "Exuma", "Grand+Bahama", "Inagua", "Long+Island", "Mayaguana", "New+Providence", "Nassau", "Ragged+Island", "Rum+Cay", "San+Salvador"}, |
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.
| "the bahamas": QueryPreset{ | |
| include: []string{"The+Bahamas", "Abaco", "Acklins", "Andros", "Berry+Islands", "Bimini", "Cat+Island", "Crooked+Island", "Eleuthera", "Exuma", "Grand+Bahama", "Inagua", "Long+Island", "Mayaguana", "New+Providence", "Nassau", "Ragged+Island", "Rum+Cay", "San+Salvador"}, | |
| "bahamas": QueryPreset{ | |
| include: []string{"Bahamas", "Abaco", "Acklins", "Andros", "Berry+Islands", "Bimini", "Cat+Island", "Crooked+Island", "Eleuthera", "Exuma", "Grand+Bahama", "Inagua", "Long+Island", "Mayaguana", "New+Providence", "Nassau", "Ragged+Island", "Rum+Cay", "San+Salvador"}, |
Without this change, someone who just puts in "Bahamas" won't be picked up.
presets.go
Outdated
| }, | ||
| "the bahamas": QueryPreset{ | ||
| include: []string{"The+Bahamas", "Abaco", "Acklins", "Andros", "Berry+Islands", "Bimini", "Cat+Island", "Crooked+Island", "Eleuthera", "Exuma", "Grand+Bahama", "Inagua", "Long+Island", "Mayaguana", "New+Providence", "Nassau", "Ragged+Island", "Rum+Cay", "San+Salvador"}, | ||
| include: []string{"The+Bahamas", "Bahamas", "Abaco", "Acklins", "Andros", "Berry+Islands", "Bimini", "Cat+Island", "Crooked+Island", "Eleuthera", "Exuma", "Grand+Bahama", "Inagua", "Long+Island", "Mayaguana", "New+Providence", "Nassau", "Ragged+Island", "Rum+Cay", "San+Salvador"}, |
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.
Hello @ashkulz 👋
I hope this recent commit represents the inclusive change required to now correctly add 'The Bahamas' to the Most Active Github Users List. Thank you!
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.
Having The Bahamas and Bahamas is redundant, so better to just have the latter.
Also, I'm not the maintainer (@lauripiispanen is) and I can just merge this in my fork https://committers.top 🤷♂️
presets.go
Outdated
| "the bahamas": QueryPreset{ | ||
| include: []string{"bahamas", "abaco", "acklins", "andros", "berry islands", "bimini", "cat island", "crooked island", "eleuthera", "exuma", "grand bahama", "inagua", "long island", "mayaguana", "new providence", "nassau", "ragged island", "rum cay", "san salvador"}, |
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.
|
Thanks for the contribution, @steveWDamesJr! I've merged it in my fork (see the commit referenced above) and the changes are live -- see ashkulz/committers.top@b1d07df. |
Thank you very much @ashkulz. Your efforts in continuing this great work by @lauripiispanen is much appreciated. Thanks also for your commitment to ensuring that 'The Bahamas' now has representation in your fork! Happy Coding 👋 |
It is realized that many Bahamas Island names are also names of states or countries around the world. To avoid skewed results, I have shortened the QueryPreset to the more unique and inclusive term of 'Bahamas'
| }, | ||
| "the bahamas": QueryPreset{ | ||
| include: []string{"Bahamas", "Abaco", "Acklins", "Andros", "Berry+Islands", "Bimini", "Cat+Island", "Crooked+Island", "Eleuthera", "Exuma", "Grand+Bahama", "Inagua", "Long+Island", "Mayaguana", "New+Providence", "Nassau", "Ragged+Island", "Rum+Cay", "San+Salvador"}, | ||
| include: []string{"Bahamas"}, |
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.
Hello 👋
After viewing the results of the live app, it is determined that the results are not an accurate depiction of users from 'The Bahamas' as users from ie. 'Long Island, NY' (which has the same name as Long Island Bahamas) are being grouped together in 'The Bahamas' Rankings.
This is true of many other Bahamas Island names that share the same name as other states or countries around the world.
To avoid skewed results, I have shortened the QueryPreset to the more unique and inclusive term of 'Bahamas'.
Please update this change in your live sites @ashkulz and @lauripiispanen.
Many thanks 👏
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.
Done, @steveWDamesJr.
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.
Add The Bahamas 🇧🇸
Thanks for your efforts 🙏