-
-
Notifications
You must be signed in to change notification settings - Fork 51
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
Unix: Fallback to default if XDG environment variable is empty #30
Conversation
Codecov Report
@@ Coverage Diff @@
## main #30 +/- ##
==========================================
+ Coverage 90.70% 93.89% +3.18%
==========================================
Files 12 13 +1
Lines 495 524 +29
Branches 44 44
==========================================
+ Hits 449 492 +43
+ Misses 32 26 -6
+ Partials 14 6 -8
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
I will need help with the failing type checks. I am able to run |
Signed-off-by: Bernát Gábor <[email protected]>
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.
I agree of inline-ing the new functionality, because we don't want to call the expesnive relatively os.path.expanduser unless we must. The tests just needed some type annotation, and you cannot use the Platformdir class directly as type because is dynamically calculated and mypy cannot deduce it. You'd need to type it as the ABC variant, but in this case we can just always run the tests on all platforms and annotate it as Unix, because the functionality you've added is tied to the Unix class, not anything else in the __init__.py
file.
Signed-off-by: Bernát Gábor <[email protected]>
Fixes #29 using https://github.com/pypa/pip/blob/867bbb00ccb0506095e19a0e111ce06ce71a2495/tools/vendoring/patches/appdirs.patch#L47-L54 as a reference implementation.
Open questions:
Todo: