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

Always use syspath conversions (#3690 split up, part 3) #4849

Merged
merged 1 commit into from
Jul 18, 2023

Conversation

wisp3rwind
Copy link
Member

@wisp3rwind wisp3rwind commented Jul 16, 2023

Follow-up to #4830 and #4848:

I've had #3690 in the pipeline for a long time; and it has grown too big to look at in one piece. This is just the first commit rebased on master. It is still quite big, but only touches test code, so should be safe (assuming CI is green).

This is part 3, with some changes to beets' core. Thus, this is probably the most sensitive PR in the series.

@wisp3rwind wisp3rwind changed the title Always use syspath conversions (#3690 split up, part 2) Always use syspath conversions (#3690 split up, part 3) Jul 16, 2023
Copy link
Member

@sampsyo sampsyo left a comment

Choose a reason for hiding this comment

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

Excellent! I can't see anything wrong here—it is comforting to know that syspath is (supposed to be) idempotent, so it's unlikely to break things too bad by calling it unnecessarily. (And anyway, all of these seem to me to qualify as "necessary.")

@sampsyo sampsyo merged commit efebd72 into beetbox:master Jul 18, 2023
@wisp3rwind wisp3rwind deleted the pr_add_syspath_3 branch July 18, 2023 20:21
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