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

Remove broken observer #400

Merged
merged 1 commit into from
Nov 9, 2015
Merged

Remove broken observer #400

merged 1 commit into from
Nov 9, 2015

Conversation

jacobwgillespie
Copy link
Member

This PR removes part of the rating button observers - they appear to be broken with the current code from Google. I've just removed them here as I'm not sure if they're necessary anymore and if so I'm not sure how to reimplement. I will say though that when combined with #389, it appears to restore LastFM scrobbling and growl notifications.

@chrismou
Copy link
Member

chrismou commented Nov 2, 2015

👍 Last.fm seems to be working again, and I'd not even noticed that notification centre notifications had stopped, but they seem to be visible now. Nice work!

What did you use to debug this, out of interest? I spent a while on it, and couldn't trigger any sort of error so was mostly feeling around in the dark.

@jacobwgillespie
Copy link
Member Author

For debugging, I enabled the WebKit debugger on the webview (so you can right-click and get to the dev tools) by editing Preferences.plist and adding a boolean key WebKitDeveloperExtras of value YES.

@chrismou
Copy link
Member

chrismou commented Nov 5, 2015

Some useful info there, cheers. I've made a note for the next time the JS starts acting weird.

Anyways, here's a LGTM / :+1: from me. All seems good from what I can see.

@chrismou
Copy link
Member

chrismou commented Nov 9, 2015

@radiant-player/radiant-player-mac Could we get some eyes on this PR when you get a chance? Would be good to get this merged in asap

@jacobwgillespie
Copy link
Member Author

I'm going to go ahead and merge with the LGTM from @chrismou - feel free to leave comments for review later and we can get them sorted.

jacobwgillespie added a commit that referenced this pull request Nov 9, 2015
@jacobwgillespie jacobwgillespie merged commit 64ac628 into master Nov 9, 2015
@jacobwgillespie jacobwgillespie deleted the fix-track-notifications branch November 9, 2015 18:47
@ebramanti
Copy link
Contributor

Removing broken code? 👍 :shipit:

@chrismou
Copy link
Member

chrismou commented Nov 9, 2015

@jacobwgillespie I love your commit stats on this repo

Added Lines: 3
Deleted Lines: 7457

😉

@kbhomes
Copy link
Member

kbhomes commented Nov 9, 2015

Great change, I'll be looking into everything in this repo later tonight.

On Mon, Nov 9, 2015 at 3:55 PM, Chris Chrisostomou <notifications@github.com

wrote:

@jacobwgillespie https://github.com/jacobwgillespie I love your commit
stats on this repo

Added Lines: 3
Deleted Lines: 7457

[image: 😉]


Reply to this email directly or view it on GitHub
#400 (comment)
.

@jacobwgillespie
Copy link
Member Author

@kbhomes awesome!

@chrismou yeah, gotta love removing lots of unnecessary code 😉

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

4 participants