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

Remove mention of python-itunes #2540

Merged
merged 2 commits into from
May 3, 2017
Merged

Remove mention of python-itunes #2540

merged 2 commits into from
May 3, 2017

Conversation

owcz
Copy link
Contributor

@owcz owcz commented May 3, 2017

The package is inactive and has been broken for months: #2371

Fixes #1610

Alternatively, could be rephrased to say it doesn't work with py3 (if that's the only condition), but I think that would cause even more confusion if there isn't a workaround

The plugin is inactive and has been broken for months: beetbox#2371

Fixes beetbox#1610
@mention-bot
Copy link

@owcz, thanks for your PR! By analyzing the history of the files in this pull request, we identified @sampsyo, @lcharlick and @Lemutar to be potential reviewers.

@sampsyo
Copy link
Member

sampsyo commented May 3, 2017

Good call; thanks. Would you mind adding a changelog entry to this effect?

@Kraymer
Copy link
Contributor

Kraymer commented May 3, 2017

Is keeping the iTunes Store section necessary ?

iTunes Store
There is currently no plugin to use the iTunes Store as an art source.

@sampsyo
Copy link
Member

sampsyo commented May 3, 2017

Yep, good point—we can probably remove that section altogether. (Beyond that, this is totally ready to merge—anybody with commit access should feel free to close it up.) Thanks again!

@Kraymer Kraymer merged commit ce48578 into beetbox:master May 3, 2017
@owcz
Copy link
Contributor Author

owcz commented May 3, 2017

I recommend keeping the section (or at least a mention) because otherwise users (like me) will wonder where fetchart's iTunes support is. iTunes' artwork is some of the most reliable on the Internet—hopefully someone will fill in this functionality soon

@owcz owcz deleted the patch-1 branch May 3, 2017 18:33
@Kraymer
Copy link
Contributor

Kraymer commented May 3, 2017

Gotcha, yet I think it's pretty clear in the first sentence that iTunes artwork is not available :

By default, this plugin searches for art in the local filesystem as well as on the Cover Art Archive, the iTunes Store, Amazon, and AlbumArt.org, in that order.
(EDITED)

Have no strong opinion though and the commit to revert is e7ea7ab5f if @sampsyo prefers.

@owcz
Copy link
Contributor Author

owcz commented May 3, 2017

As far as I can tell, as currently merged, https://github.com/beetbox/beets/blob/master/docs/plugins/fetchart.rst doesn't include that sentence or any other mention of iTunes

@Kraymer
Copy link
Contributor

Kraymer commented May 3, 2017

@owcz
Copy link
Contributor Author

owcz commented May 3, 2017

Right, but not with "iTunes"—the point is that "iTunes" isn't mentioned at all on the page so, as written, someone looking for iTunes fetchart compatibility has no idea what to do next. Anyway, your call on how to resolve, though I suggest giving users (such as myself) a heads up within the documentation as that's where I went to look first (before finding that the plugin indeed no longer works)

sampsyo added a commit that referenced this pull request May 4, 2017
@sampsyo
Copy link
Member

sampsyo commented May 4, 2017

Thanks for editing the docs, @owcz, and for merging, @Kraymer.

Our usual MO in these situations is to advertise the change loudly and clearly in the changelog instead of in the documentation pages themselves. There's of course a trade-off, but keeping the core documentation changes free of historical cruft has been helpful in making them clear for new users, who just want to see what's currently there. I think our announcement in the changelog could still be louder and clearer, but we'll get there.

@Kraymer
Copy link
Contributor

Kraymer commented May 5, 2017

@owcz in the meantime, if iTunes fetchart support is something you want back, you could open a dedicated feature request issue that would bring visibility

nathdwek pushed a commit that referenced this pull request Aug 15, 2018
nathdwek pushed a commit that referenced this pull request Aug 15, 2018
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.

4 participants