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

Cleanup: remove no-longer-referenced get_known_folder_path, UserDirs class #227

Merged
merged 2 commits into from
Jan 7, 2023
Merged

Cleanup: remove no-longer-referenced get_known_folder_path, UserDirs class #227

merged 2 commits into from
Jan 7, 2023

Conversation

jayaddison
Copy link

While looking at a similar build failure to the one mentioned in #218, I noticed that the UserDirs class (XDG-related functionality) where a translation-retrieval exception occurred may be part of an unused section of code.

As far as I can tell, the UserDirs class is only referenced by the get_known_folder_path method, which is imported -- but not invoked -- by worker.py.

I ran most tests for this locally, although I had some difficulty getting some of the tests to run in a containerized environment (so I'm leaning on continuous integration a bit, here).

(note for maintainers: this changeset currently applies cleanly to the develop branch, if that's a better place to direct this change)

@jayaddison
Copy link
Author

Hm, I guess continuous integration didn't run for this pull request. See the downstream branch here for a demonstration of the test results before and after the change from 015753b is applied: https://github.com/jayaddison/displaycal-py3/commits/develop

Copy link
Collaborator

@p5k369 p5k369 left a comment

Choose a reason for hiding this comment

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

Hey @jayaddison,
you are right. That function was used by the original author around 2017 and never since. I think it can be removed.
However, please do not commit to the main branch and instead to the develop branch.

@jayaddison jayaddison changed the base branch from main to develop January 7, 2023 23:14
@jayaddison
Copy link
Author

Thank you, @p5k369 - merge branch updated.

Copy link
Collaborator

@p5k369 p5k369 left a comment

Choose a reason for hiding this comment

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

Hey @jayaddison thank you for your pr

@p5k369 p5k369 merged commit 5ad1ac4 into eoyilmaz:develop Jan 7, 2023
@jayaddison
Copy link
Author

You're welcome! Thank you for the merge.

@jayaddison jayaddison deleted the issue-218/tangential-xdg-cleanup branch January 7, 2023 23:46
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.

3 participants