Skip to content
This repository was archived by the owner on May 16, 2023. It is now read-only.

Conversation

@steveWDamesJr
Copy link

@steveWDamesJr steveWDamesJr commented Apr 1, 2022

Add The Bahamas 🇧🇸

  • Please find the queryPreset "The Bahamas" within presets.go

Thanks for your efforts 🙏

Add list of principal islands in The Bahamas
@steveWDamesJr steveWDamesJr changed the title This branch is to add "The Bahamas" and principal islands queryPreset in presets.go This branch is to add "The Bahamas" and principal islands queryPreset in presets.go 🇧🇸 Apr 1, 2022
Copy link

@ashkulz ashkulz left a 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
Comment on lines 386 to 387
"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"},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"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"},
Copy link
Author

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!

Copy link

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
Comment on lines 386 to 387
"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"},
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @ashkulz for the tip.

I have made the change to the pull request.

Happy coding! 👋

ashkulz added a commit to ashkulz/committers.top that referenced this pull request Jun 29, 2022
@ashkulz
Copy link

ashkulz commented Jun 29, 2022

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.

@steveWDamesJr
Copy link
Author

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"},
Copy link
Author

@steveWDamesJr steveWDamesJr Jun 29, 2022

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 👏

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @ashkulz 👋 ,

Thank you for your quick action in resolving this issue.

Best,

Steve

@steveWDamesJr steveWDamesJr changed the title This branch is to add "The Bahamas" and principal islands queryPreset in presets.go 🇧🇸 This branch is to add "The Bahamas" queryPreset in presets.go 🇧🇸 Jun 29, 2022
ashkulz added a commit to ashkulz/committers.top that referenced this pull request Jun 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants