Skip to content
This repository has been archived by the owner on Mar 30, 2021. It is now read-only.

Fix for random unloving/loving of tracks on last.fm #426

Merged
merged 1 commit into from
Dec 1, 2015

Conversation

chrismou
Copy link
Member

See #390

Essentially, the rating observer was being called before the track info was updated, meaning that for every song it was applying the loved status of the previous song. This PR should fix that.

I've logged a separate issue at #425 to look into stopping the multiple unnecessarily api calls at the start of every song.

@jacobwgillespie
Copy link
Member

LGTM (though separately we should probably change the CHANGELOG on each PR, so you may want to add an entry)

:shipit:

@chrismou
Copy link
Member Author

chrismou commented Dec 1, 2015

Ah OK, so we're adding our own changelog entries? No worries, I'll update it before I merge.

chrismou added a commit that referenced this pull request Dec 1, 2015
Fix for random unloving/loving of tracks on last.fm
@chrismou chrismou merged commit 67f1dd9 into radiant-player:master Dec 1, 2015
@chrismou chrismou deleted the lastfm-unlove branch December 1, 2015 08:44
@chrismou
Copy link
Member Author

chrismou commented Dec 1, 2015

Interesting point though - if we're referencing PRs from the changelog (which is a good idea), this means we have to PR to get the PR url, then update the changelog, rebase, and push an updated PR straight away. Which isn't the end of the world, but it is a bit of a PITA. It also means we have to ask contributers to update the changelog.

tbh, I wouldn't have an issue with the merger pushing a changelog change directly to master once it's been merged, as it's not a functional change. As long as it's just a change to that file, it shouldn't be an issue and it'll only ever be one of us that does it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants