Skip to content

Conversation

@tamarl08
Copy link
Contributor

@tamarl08 tamarl08 commented Sep 9, 2024

If None is given in the configuration file, it is passed as the string "None". This change converts it to "none" which is the default value.
I didn't change anything in the documentation - @desilinguist do you None this should be explicitly mentioned as an acceptable option?

@pep8speaks
Copy link

pep8speaks commented Sep 9, 2024

Hello @tamarl08! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-09-09 16:17:15 UTC

@desilinguist
Copy link
Collaborator

I didn't change anything in the documentation - @desilinguist do you None this should be explicitly mentioned as an acceptable option?

@tamarl08 yes I think we should. Not only that None is now an alias for "none"/"None" but that other options are also case-insensitive.

@codecov
Copy link

codecov bot commented Sep 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.40%. Comparing base (51c540b) to head (79899ff).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #775   +/-   ##
=======================================
  Coverage   95.40%   95.40%           
=======================================
  Files          29       29           
  Lines        3676     3676           
=======================================
  Hits         3507     3507           
  Misses        169      169           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@desilinguist desilinguist left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@tamarl08 tamarl08 merged commit 231867d into main Sep 11, 2024
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.

5 participants