search

msfeldstein / chrome-media-keys

Chrome extension that adds media keys to most web music players
GNU Affero General Public License v3.0
89 stars 33 forks source link

Convert To Basic #57

Open nemchik opened 9 years ago

nemchik commented 9 years ago

There are 20+ non basic controllers as of posting this issue. It might be a good idea to investigate which ones can be converted to basic to make long term maintenance easier. This might be better as a readme or wiki page that can be updated over time as controllers are converted to basic. Here is a list of all non basic controllers;

BronyTunes GetWorkDone Hypemachine Lastfm Monstercat PonyFM PonyvilleLive Qriosity Rbma Shim (isn't this the Unity API?) Soundtracker Sway Thesixtyone ThisIsMyJam

nemchik commented 9 years ago

Removed Netflix https://github.com/msfeldstein/chrome-media-keys/pull/58 and Spotify https://github.com/msfeldstein/chrome-media-keys/pull/56

mbirth commented 8 years ago

Despite YouTube I have no account at any of those services so I couldn't check if the converted controller is working. Some look easy enough that the conversion should be possible "blind", but I wouldn't want to risk breaking things.

Ustice commented 7 years ago

There are a bunch of really old controllers. I removed the ones that are for services that are defunct in #104, but I am sure that there are a bunch that no longer work. I would be good to do an audit of the controllers to see which no longer work, and prioritize the services.

Since Google Analytics is included already, I would suggest that we add it to the BasicController to see which services are actually used.

nemchik commented 7 years ago

I completely agree about analytics, but is the base controller the right place to put it? Wouldn't that lack information about the controllers that are not basic?

Ustice commented 7 years ago

Is there a reason to not base all of the controllers on the BasicController while we are doing this?

msfeldstein commented 7 years ago

Yeah BasicController was added later, there are some legacy controllers that don't use it, but there's no reason that they can't be converted to use it. If there's some reason a certain service can't use BasicController file an issue and we can augment BasicController

On Sat, Dec 3, 2016 at 5:19 PM Jason Kleinberg notifications@github.com wrote:

Is there a reason to not base all of the controllers on the BasicController while we are doing this?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/msfeldstein/chrome-media-keys/issues/57#issuecomment-264677434, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJ1b-0ehcJsj0w2GCG7aEXiMTCrSV2dks5rEhU2gaJpZM4Ejwjy .

nemchik commented 7 years ago

I updated the list to remove the controllers dropped with #104

The only reason any of the remaining controllers are still not basic is either I didn't have access to the site to test a conversion, or I attempted a conversion and couldn't get it quite right.

I agree that blind conversions are possible, but risky. If anyone else wants to finish off the list to convert, feel free.