Skip to content
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

A Naming Convention Issue on Australian States #210

Closed
hugowebtools opened this issue Mar 12, 2020 · 3 comments · Fixed by #214
Closed

A Naming Convention Issue on Australian States #210

hugowebtools opened this issue Mar 12, 2020 · 3 comments · Fixed by #214
Assignees

Comments

@hugowebtools
Copy link

hugowebtools commented Mar 12, 2020

I was trying to create two objects for Victoria and New South Wales in Australia and I realised the the holiday providers for these two states were using different naming conventions.

$provider = Yasumi::create('Australia/NSW', 2020);
$provider = Yasumi::create('Australia/Victoria', 2020);

If we check the list of providers, we can easily see there are 5 states in Australia using their abbreviations as the class name against the others.
Screen Shot 2020-03-12 at 16 04 08
I don't think it is correct. Based on the naming conventions used for states in Germany, maybe we can simply align Australia states to reduce confusion.

   NSW  =>  NewSouthWales
   ACT  =>  AustralianCapitalTerritory
   WA => WesternAustralia
   SA => SouthAustralia
   NT => NorthernTerritory
@Milamber33
Copy link
Contributor

As the guy who actually built all of the Australian providers, my apologies. :)
Personally, I'm using Yasumi::createByISO3166_2 rather than just Yasumi::create, so if someone feels like changing this I am not opposed.

@hugowebtools
Copy link
Author

Ah, it is. I can see Yasumi::createByISO3166_2 a cool feature and it is very useful. I must miss it in the documentation. Thank you.

@stelgenhof
Copy link
Member

To have consistency, renaming them would be good indeed. I can make that change if you like, other otherwise feel free to submit a PR :)

Cheers! Sacha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants