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

iTunes: Move path mapping logic into XML importer and clean up slightly #11446

Merged
merged 3 commits into from
Apr 16, 2023

Conversation

fwcd
Copy link
Member

@fwcd fwcd commented Apr 5, 2023

This allows for cleaner separation of concerns, since the path mapping is not needed in the macOS importer (and potentially others in the future). Also, since the mapping is applied by the XML importer itself, having the iTunes feature own it at a higher level doesn't really prove beneficial.

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

Thank you. LGTM

@Swiftb0y
Copy link
Member

Swiftb0y commented Apr 7, 2023

@daschuer since you already have an itunes testing setup, would you like to test this PR as well or can we be sure enough to work by looking at the code changes?

@fwcd
Copy link
Member Author

fwcd commented Apr 7, 2023

We should probably add a unit test for an iTunes XML so we can refactor safely without having to worry as much about regressions. But I'd move that to a future PR too.

@Swiftb0y
Copy link
Member

Moving forward. Regressions can be filed during beta.

@Swiftb0y Swiftb0y merged commit ce063e0 into mixxxdj:2.4 Apr 16, 2023
@fwcd fwcd deleted the move-itunes-path-mapping-to-xml branch April 16, 2023 21:51
@fwcd fwcd added the itunes label Oct 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants