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

Change core.history.add() signature #1056

Closed
jodal opened this issue Mar 19, 2015 · 3 comments · Fixed by #1063
Closed

Change core.history.add() signature #1056

jodal opened this issue Mar 19, 2015 · 3 comments · Fixed by #1063
Assignees
Labels
A-core Area: Core layer C-enhancement Category: A PR with an enhancement or an issue with an enhancement proposal

Comments

@jodal
Copy link
Member

jodal commented Mar 19, 2015

Take uri, name instead of a track that is simply converted to a track ref.

This makes the API also work for the use case of adding stream titles to the history.

@jodal jodal added C-enhancement Category: A PR with an enhancement or an issue with an enhancement proposal A-core Area: Core layer labels Mar 19, 2015
@jodal jodal added this to the v1.0 - Audio cleanup 1 milestone Mar 19, 2015
@adamcik
Copy link
Member

adamcik commented Mar 20, 2015

Other thing regarding this, the add API should not be public. History is read only for anything outside of core.

@jodal
Copy link
Member Author

jodal commented Mar 20, 2015

As in _add() or just internal through documentation?

@adamcik
Copy link
Member

adamcik commented Mar 20, 2015

As in _add IMO

jodal added a commit to jodal/mopidy that referenced this issue Mar 20, 2015
Instead of changing the signature to add(uri, name) I opted for
renaming it to _add_track(track).

Since it's internal we may change it whenever we like to. Since you need
different logic for extracting an interesting name from a track and from
a ref or a stream title, it makes sense to add another method for adding
refs/stream titles to the history when that time comes.

Fixes mopidy#1056
@jodal jodal self-assigned this Mar 20, 2015
@jodal jodal added 3 - Done and removed 1 - Ready labels Mar 20, 2015
jodal added a commit to jodal/mopidy that referenced this issue Mar 20, 2015
Instead of changing the signature to add(uri, name) I opted for
renaming it to _add_track(track).

Since it's internal we may change it whenever we like to. Since you need
different logic for extracting an interesting name from a track and from
a ref or a stream title, it makes sense to add another method for adding
refs/stream titles to the history when that time comes.

Fixes mopidy#1056
jodal added a commit to jodal/mopidy that referenced this issue Mar 20, 2015
Instead of changing the signature to add(uri, name) I opted for
renaming it to _add_track(track).

Since it's internal we may change it whenever we like to. Since you need
different logic for extracting an interesting name from a track and from
a ref or a stream title, it makes sense to add another method for adding
refs/stream titles to the history when that time comes.

Fixes mopidy#1056
@jodal jodal removed the 3 - Done label Mar 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Core layer C-enhancement Category: A PR with an enhancement or an issue with an enhancement proposal
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants