-
Notifications
You must be signed in to change notification settings - Fork 0
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
Upgrade countries gem to 5.7 from 2.x #5
Conversation
Huge version change as the countries gem is extremely out of date. This commit is an attempt to simply upgrade it, and see what breaks.
166577d
to
5faf8da
Compare
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.
Great work! 🙌🏼
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.
Nice work!
It'd probably be a good idea to update the countries gem in the Charlie codebase too, as I can see we take countries from the national holidays gem and use them in the Phi process.
I think the update would be pretty straightforward there too, as I think we only really use the #name
method
Mhm, that was step 2 @WillRogers727, and yeah it should be somewhat simple to sort out once we're no longer held to 2.x by this gem. Currently waiting on gem publishing permissions from Alex before I'll merge 👍 |
Update: have been given permissions, will be merging in PR and following README to publish 2.0 of |
Huge version change as the countries gem is extremely out of date.
The changelog for the countries gem describes needing an upgrade process for the upgrade to 4.2, and for the upgrade to 5.0. As such, I've made a major version change to this change, to reflect that these were breaking changes and may cause issues elsewhere.
The only breaking change within this gem itself for
national-holidays-ruby
is that.name
is now deprecated on the countries gem. This means that instead, we're using.iso_short_name
. There is also.common_name
, and.iso_long_name
, but these do not match the previous expected behaviours. Common name shortensUnited Kingdom of Great Britain and Northern Ireland
to simplyUnited Kingdom
, whereas long names addThe
to country names..iso_short_name
is the closest to existing functionality.The other breaking change for 5.0 breaks
find_by
methods, but these are unused here.UPGRADE.md for the countries gem: https://github.com/countries/countries/blob/master/UPGRADE.md
Changelog for countries gem: https://github.com/countries/countries/blob/master/CHANGELOG.md