Fix the remaining TODOs #257
Conversation
marioortizmanero
force-pushed
the
todos
branch
from
0c5f411
to
7bc0d6d
Compare
marioortizmanero
force-pushed
the
todos
branch
from
f8bc323
to
10dfa4b
Compare
CHANGELOG.md
Outdated
| + `playlist_replace_tracks` is now `playlist_replace_items` | ||
| + `playlist_reorder_tracks` is now `playlist_reorder_items` | ||
| + `playlist_remove_all_occurrences_of_tracks` is now `playlist_remove_all_occurrences_of_items` | ||
| + `playlist_remove_specific_occurrences_of_tracks` is now `playlist_remove_specific_occurrences_of_tracks` |
There was a problem hiding this comment.
playlist_remove_specific_occurrences_of_tracksis nowplaylist_remove_specific_occurrences_of_tracks
It should be playlist_remove_specific_occurrences_of_tracks is now playlist_remove_specific_occurrences_of_items?
There was a problem hiding this comment.
Yup that's a typo
rspotify-model/src/track.rs
Outdated
| @@ -92,7 +92,7 @@ pub struct SavedTrack { | |||
| } | |||
|
|
|||
| /// Track id with specific positions track in a playlist | |||
| pub struct TrackPositions { | |||
| pub struct ItemPositions { | |||
There was a problem hiding this comment.
Since you have renamed TrackPositions to ItemPositions, should we change the id field to something like PlayableId(ItemId)?
And we should update the doc above.
There was a problem hiding this comment.
OK, so I've modified ItemPositions so that it uses borrowed data instead of owned, including &dyn PlayableId instead of Box<dyn PlayableId>. This is because ItemPositions is only passed as a short-lived wrapper and doesn't really need to hold data, in my opinion. This way we can avoid two unnecessary allocations. If that's OK with you you can merge. Otherwise we can make it owned.
Description
headerstoparamsURI_WRONGTYPE2Motivation and Context
We can finally close #127 after this, believe it or not! :))
Dependencies
None
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
CI passes successfully