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

m3u: Extract new M3U backend from local #1066

Merged
merged 1 commit into from Mar 21, 2015

Conversation

3 participants
@jodal
Member

jodal commented Mar 21, 2015

Fixes #1054

@jodal jodal added this to the v1.0 - Audio cleanup 1 milestone Mar 21, 2015

@@ -24,7 +24,7 @@ def get_config_schema(self):
schema['library'] = config.String()
schema['media_dir'] = config.Path()
schema['data_dir'] = config.Path()
schema['playlists_dir'] = config.Path()
schema['playlists_dir'] = config.Deprecated()

This comment has been minimized.

@adamcik

adamcik Mar 21, 2015

Member

Probably not worth it in this case, but we could probably also do something like to a config.Deprecated(msg='...') or config.Deprecated(new='m3u/data_dir') to ease migration.

This comment has been minimized.

@jodal

jodal Mar 21, 2015

Member

Doesn't look like we currently do anything with config.Deprecated() except not throwing a fit over finding an unknown config value in the file. Feels out of scope for this PR, but surely something we should consider adding.

Editing playlists
=================
There is a core playlist API in place for editing playlists. Though, we are not

This comment has been minimized.

@tkem

tkem Mar 21, 2015

Member

"not aware of any frontends or clients" is probably gonna change soon (couple of weeks), so this is bound to become obsolete. So I'd suggest to remove that sentence, or rephrase it like "If your client does not support playlist editing..." or "to edit playlist files manually..." or something.

If the M3U extension should be enabled or not.
.. confval:: m3u/data_dir

This comment has been minimized.

@tkem

tkem Mar 21, 2015

Member

I liked playlists_dir better. data_dir to me sounds too generic, or too much like "internal data" I shouldn't mess with from the command line. But that's nitpicking.

@tkem

This comment has been minimized.

Member

tkem commented Mar 21, 2015

IMHO, M3UPlaylistsProvider._playlists should be converted to {uri: playlists}. Makes several things much easier and clearer. This was also part of #1053, though I should have probably better split that into two PRs.

@jodal

This comment has been minimized.

Member

jodal commented Mar 21, 2015

Yes, probably. I haven't changed anything yet, just split it.

@tkem

This comment has been minimized.

Member

tkem commented Mar 21, 2015

Sure. I could also rebase/repost #1053 on top of this, just can't say if i can manage it "this week".

@jodal jodal force-pushed the jodal:feature/add-m3u-backend branch from 0449edf to 6a10171 Mar 21, 2015

@jodal jodal force-pushed the jodal:feature/add-m3u-backend branch from 6a10171 to b2f60bc Mar 21, 2015

@jodal

This comment has been minimized.

Member

jodal commented Mar 21, 2015

That would be great! Also possible I'll have time to have a look at reapplying it on top of this.

@tkem

This comment has been minimized.

Member

tkem commented Mar 21, 2015

Any way, shouldn't delay v1.0 IMO, since this is purely an implementation detail. So, from my part, go ahead with this ;-)

adamcik added a commit that referenced this pull request Mar 21, 2015

Merge pull request #1066 from jodal/feature/add-m3u-backend
m3u: Extract new M3U backend from local

@adamcik adamcik merged commit 29f8ee0 into mopidy:develop Mar 21, 2015

3 checks passed

Scrutinizer 48 updated code elements
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.02%) to 76.45%
Details

@adamcik adamcik self-assigned this Mar 21, 2015

@jodal jodal deleted the jodal:feature/add-m3u-backend branch Mar 21, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment