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

Rework beets.util.hidden #5341

Merged
merged 5 commits into from
Jul 11, 2024
Merged

Rework beets.util.hidden #5341

merged 5 commits into from
Jul 11, 2024

Conversation

bal-e
Copy link
Member

@bal-e bal-e commented Jun 27, 2024

The implementation was unnecessarily convoluted. I've rewritten it to declare fewer functions, and it now uses pathlib.

P.S. I'm starting to feel bad for opening so many PRs :P

The implementation was unnecessarily convoluted.  I've rewritten it to
declare fewer functions, and it now uses 'pathlib'.
Copy link

Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.

Arav K. added 3 commits June 27, 2024 15:05
This check existed in the original implementation, but I had removed it
due to a misinterpretation of the 'os' documentation, thinking that the
field was guaranteed to exist.
@Serene-Arc
Copy link
Contributor

Don't feel bad! It's great to see someone interested in the repo and contributing :)

Copy link
Contributor

@Serene-Arc Serene-Arc left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Looks good mostly, one little nitpick.

beets/util/hidden.py Outdated Show resolved Hide resolved
@bal-e
Copy link
Member Author

bal-e commented Jul 6, 2024

bump -- let me know if there's anything else left to do.

@Serene-Arc Serene-Arc merged commit 9122722 into beetbox:master Jul 11, 2024
12 checks passed
@bal-e bal-e deleted the rework-hidden branch July 11, 2024 21:06
@bal-e
Copy link
Member Author

bal-e commented Jul 11, 2024

Thanks @Serene-Arc!

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