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

433: Added if checks around class definitions. #414

Merged
merged 3 commits into from
Apr 20, 2016

Conversation

toolstack
Copy link
Contributor

This is to avoid conflicts with other plugins that may include locales.php for their own use.

Alternate if syntax used to avoid triggering coding standards review of entire class.

Resolves #413.

This is to avoid conflicts with other plugins that may include locales.php for their own use.

Alternate if syntax used to avoid triggering coding standards review of entire class.
@toolstack toolstack added this to the 2.0.1 milestone Apr 19, 2016
@ocean90
Copy link
Member

ocean90 commented Apr 19, 2016

The path to locales.php can be changed via https://github.com/GlotPress/GlotPress-WP/blob/2.0.0/gp-settings.php#L4 which means that custom locales.php need to have the class_exists() too.
What do you think about adding a class_exists() check to https://github.com/GlotPress/GlotPress-WP/blob/2.0.0/gp-settings.php#L57?

@toolstack
Copy link
Contributor Author

I considered it but thought it was better inside the class file as if another plugin pulls a copy of it use then we make sure it doesn't break.

We could add the calls_exists() to both places but I didn't like the code duplication.

This makes sure that if a custom locales.php file has been used and doesn't have the required class checks, it won't break things.
@ocean90 ocean90 self-assigned this Apr 20, 2016
@ocean90 ocean90 merged commit 4578dda into develop Apr 20, 2016
@ocean90 ocean90 deleted the 413-add-check-for-locales-class branch April 20, 2016 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants