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

Fix #1259 by respecting core dir configs #1266

Merged
merged 4 commits into from Aug 23, 2015

Conversation

2 participants
@jodal
Member

jodal commented Aug 22, 2015

No description provided.

@jodal jodal added this to the v1.1.1 - Bugfixes milestone Aug 22, 2015

@jodal jodal force-pushed the jodal:fix/1259-respect-core-dir-configs branch from c8013ff to 49bc710 Aug 22, 2015

jodal added a commit to jodal/mopidy that referenced this pull request Aug 22, 2015

@jodal

This comment has been minimized.

Member

jodal commented Aug 22, 2015

Amended to fix tests.

@jodal jodal force-pushed the jodal:fix/1259-respect-core-dir-configs branch from 4523cdd to 369f10b Aug 22, 2015

@adamcik

This comment has been minimized.

Member

adamcik commented Aug 23, 2015

Why is one becoming none, and the other being deprecated? Shouldn't we aim for consistency?

I guess this has to do with the local extensions using the dir as well?

@jodal

This comment has been minimized.

Member

jodal commented Aug 23, 2015

I removed local/data_dir since it feels like a duplicate of core/data_dir and Extension.get_data_dir().

m3u/playlists_dir I consider to more alike local/media_dir, which we still have. It defaults to the extension's data dir, but it would be quite common for users to point it somewhere else entirely, as they probably care more for their playlists and where they are stored (to be used in other apps too?) than they care about where the Mopidy-Local metadata index is stored.

When we split Mopidy-M3U out of Mopidy-Local, @tkem (IIRC) actually made a point of keeping the "playlists_dir" config value name instead of renaming it to m3u/data_dir, like I initially did.

@adamcik

This comment has been minimized.

Member

adamcik commented Aug 23, 2015

Right, makes sense. Just wanted to check :-)

adamcik added a commit that referenced this pull request Aug 23, 2015

@adamcik adamcik merged commit 2d46a73 into mopidy:v1.1.x Aug 23, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.03%) to 78.009%
Details

@jodal jodal deleted the jodal:fix/1259-respect-core-dir-configs branch Aug 23, 2015

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