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

Config: fix erroneous check of KIVY_NO_ENV_CONFIG #6540

Merged
merged 1 commit into from
Sep 28, 2019

Conversation

fluxrider
Copy link
Contributor

The check to see if KIVY_NO_ENV_CONFIG is set was incorrect.

Without this fix, one has to set os.environ["KIVY_NO_ENV_CONFIG"] = "1" to be able to use KCFG_KIVY_LOG_LEVEL=warning and the likes. Which is the opposite of what config.py's doc says.

@welcome
Copy link

welcome bot commented Sep 27, 2019

Thanks for opening your first pull request here! 💖 Please check out our contributing guidelines.

Copy link
Member

@gottadiveintopython gottadiveintopython left a comment

Choose a reason for hiding this comment

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

Looks ok to me

kivy/config.py Outdated
@@ -906,7 +906,7 @@ def name(self, value):
Logger.exception('Core: Error while saving default config file')

# Load configuration from env
if environ.get('KIVY_NO_ENV_CONFIG', '0') != '0':
if environ.get('KIVY_NO_ENV_CONFIG', '0') == '0':
Copy link
Member

@matham matham Sep 28, 2019

Choose a reason for hiding this comment

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

It should actually be if environ.get('KIVY_NO_ENV_CONFIG', '0') != '1':, because if it's empty it shouldn't disable it. Only if it's explicitly 1 should it be disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's alright. I went the C style if not zero then it's set, but it would allow people to be sloppy (a bit like how it was before by just checking if the variable existing in env instead of checking the value). Cheers.

@matham matham merged commit f223133 into kivy:master Sep 28, 2019
@welcome
Copy link

welcome bot commented Sep 28, 2019

Congrats on merging your first pull request! 🎉🎉🎉

@fluxrider
Copy link
Contributor Author

Thanks, it's always fun to contribute even if it's very small patch like this one.

@matham matham added this to the 2.0.0 milestone Oct 28, 2020
@matham matham changed the title fix erroneous check of KIVY_NO_ENV_CONFIG Config: fix erroneous check of KIVY_NO_ENV_CONFIG Dec 8, 2020
@matham matham added the Component: core-app app, clock, config, inspector, logger, resources, modules, clock, base.py label Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: core-app app, clock, config, inspector, logger, resources, modules, clock, base.py
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants