twolfson / google-music.js

Browser-side JS library for controlling Google Music
The Unlicense
47 stars 6 forks source link

Move this repo into a shared space and expand into a bigger "framework" #13

Closed MarshallOfSound closed 8 years ago

MarshallOfSound commented 8 years ago

Copy paste from the message I sent a while back

Hey guys, I added both of you to this group as I think we are all "stakeholders" (if that is the correct word to use), in the potential for an expanded Google Play Music JS Framework that extends on what @twolfson has already done in his Google Music node package.

I have recently made a branch on my player that changes all my JS to use the Google Music node package, but I realized while testing that what I was doing had a few issues a) My code was still fragmented, some of it used this marvelous API. Whilst the rest was doing everything manually with obscure, unrecognizable selectors b) I had no control over this framework, if something broke I couldn't push out a fix in a few hours I would have to wait for either @twolfson to fix it or submit a PR and hope it gets merged c) These fixes still relied on the user updating their application, if for whatever reason they decided not too (some users are stubborn like that) they will not get the fixes...

I would like to propose the following We should either fork or move the Google Music node repository into a space where people who use it (Me, Radiant Player, Electron Player, Etc.) will have contributor rights to allow one of us to push fixes ASAP. This will allow changes to go out much quicker as it will take only one of us to fix everyone's applications Instead of depending upon statically released JS files we should depend on a CDN? Or the latest github release? so that updates to this framework, can be pushed independently of a version bump on our players This proposed framework should maintain the existing functionality of the current Google Music node package but should also have add on packages that allow theming support (I have some funky JS in my repo at the moment that does this) and other things like possibly a central last.fm intergration library? What do you guys think of this proposal?

Tagging relevant parties @twolfson @jacobwgillespie @chrismou

twolfson commented 8 years ago

First off, thanks for raising this issue. It's good that we are finally trying to unify our efforts with respect to handling the brittle DOM of Google Music.

I will talk to each about points individually.

Fragmented code

With respect to using a common API but it not encompassing the entirety of your app, this is kind of like chasing The Holy Grail. We are different applications that have some overlap but are not duplicates of each other.

For example, Radiant has theming and hides Google notification whereas I choose to avoid the potential brittle-ness and keep the UI more/less the same.

That being said, there is definitely some more room to help each other out (as you say -- last.fm integration would be nice). However, I don't think an overarching framework is the way to do that. Instead, I would prefer more one-off repos (so I can only download/import what I need) and maybe a larger library that allows you to bind them all at once (if that floats your boat).

Common ownership

I'm all for having common ownership. We have been wanting that for a long while (radiant-player/radiant-player-mac#252).

I think that the argument about something breaking without a quick turnaround is a bit of a strawman. When Google Music shifted to Material, we updated our integration within 24 hours.

While I see your concern, I also don't want to maintain a repository that is being cowboy committed (i.e. without a code review process). My suggestion would be something like:

I should also mention that npm supports referencing git URLs. This means we can publish our application with a non-published google-music.js.

https://docs.npmjs.com/files/package.json#dependencies

https://docs.npmjs.com/files/package.json#git-urls-as-dependencies

Fixes rely on users updating their applications

While I understand your concern, I think this is attempting to solve an effect of a deeper problem. There's usually a reason for people refusing to update (e.g. nothing telling them about new update, not willing to take time to update). For that, I would suggest finding out why and maybe adopting an auto-updater (e.g. Squirrel -- what's used by Electron):

https://github.com/Squirrel

Additionally, if we were to always point our JS to master, then it will only work until we introducing a breaking change. At some point, our API might change; we might rename a method or change a function signature for ease of use or consistency. We don't want to code ourselves into a corner which will break everyone =/

One alternative would be to have major/minor branches as part of updates (e.g. for a release like 1.2.3, we would update a branch 1.x.x and 1.2.x which point to the same version). That could be used with http://rawgit.com/. I use the same technique on gratipay-badge:

https://github.com/gratipay/gratipay-badge

tl;dr

To summarize everything:

jacobwgillespie commented 8 years ago

Here's my thoughts:

Fragmented code

I agree with @twolfson, I like the idea of multiple low-level modules with a common integration module. IDK if this should be one repo (with multiple modules within it) or multiple repos.

Common ownership

Common ownership is great. I like the argument for it based on fast turnaround, though I would phrase it more as "bus factor". As in, the library shouldn't depend on a single developer's time.

I completely agree about cowboy-committed repos. With Radiant we're doing pretty much what you've described where somebody opens a PR, then another collaborator does a code review, then the original submitter merges after approval. It's more or less documented at https://github.com/radiant-player/radiant-player-mac/blob/master/CONTRIBUTING.md.

Updating applications

I like the idea of having proper semver and publishing 1.x, 2.x versions that are live-loaded over HTTPS, since then we can push fixes while not breaking backwards compatibility. Eventually we'd stop pushing fixes to older branches, so at some point users would need to update their apps, and for that I like the emphasis on each individual app implementing an easy / auto updating system.

The org

I'm not sure where you all think this should be moved. I'd suggest @radiant-player, though my main concern there is currently @kbhomes is the only admin on that org, so any admin-related tasks would have to run through him (bus-factor issue). I'll see if he's open to adding at least one more admin to the org, which would eliminate that concern. Or perhaps a new org should be created just for this project...

chrismou commented 8 years ago

Hey guys

As I mentioned on a previous conversation, I'm definitely up for more pooling of resources. They way I see it, the more that can be abstracted away to a centralised repository the better, while leaving it up to each individual application to worry about what other features they provide. A good example is I've just spotted (and PR'd for radiant player) that Google look to have changed their song title element ID from #player-song-title to #currently-playing-title, breaking scrobbling and displaying all song title notifications as being "undefined". Course there's no way to avoid Google's changes, there's a good chance that the both of you will either a) Have already noticed and updated to reflect this change, or b) Now need to check to see if you're affected.

But yeah, minor tweaks aside, there's definitely scope for some of the suggestions here. last.fm integration is a biggie for me - I've fixed several issues on radiant player, and I'm exploring the possibility of moving the entire process out to the JS side of the app (right now it calls an Objective-C service). And I've started work on a POC for last.fm integration for @twolfson's Google Music Electron which, inevitably, will eventually also make it's way into radiant player if @jacobwgillespie's electron branch of Radiant Player ever takes off. It does feels like I'm doing the same thing multiple times over in slightly different ways.

App updates are probably something that should be left to the individual app. Radiant I think uses the sparkle framework, which is pretty seamless. I doubt we'll want to move away from that any time soon.

But one thing we've definitely got down now since moving to an organisation is the peer review stuff. I recently added a contributing doc (https://github.com/chrismou/radiant-player-mac/blob/master/CONTRIBUTING.md) which we've stuck to fairly religiously so far. No merges without at least one OK from another contributor, and for biggies I've been looking for at least 2, squashing commits for PRs, no functional changes committed directly to master, etc. It's a process that's worked well so far, and can only get better if there are more active developers involved (which, from what I've seen, all you guys are pretty damn responsive when it comes to fixes/PRs on your respective repos).

So yeah, I feel like there's definitely scope for some sort of org to divvy out some of the repeated jobs to make all the apps more stable, but really it comes down to what we could make generic enough for it to be worth centralising. google-music.js does what it does really well, last.fm integration would be great but I'd have to think about how this can be made generic enough for it to be worth it. At the end of the day, it's going to involve just pulling in an existing last.fm library (there are several good ones) and just writing a wrapper to interact with the relevant selectors. Theming could be a biggie, and I'd happily get stuck in with helping develop that, but again, there'd need to be discussion on how this'd be implemented in a way that would be useful to all of us (and any other apps that want to use it). Radiant's theming is good in some ways, but a bit of a pain in others - like the styles built by the Objective-C side of the app, and requiring a restart to apply the new theme. There's a lot of room for improvement there.

So yeah, the tl;dr - if you guys are up for that discussion, then I'm certainly on board. It'd be nice to get through a week without having to investigate why something's broken and spending what time I have available working on new features! ;-)

chrismou commented 8 years ago

Ha! It does make me laugh at despite being an 8 hour time difference apart, 3 of us have chipped in within an hour of each other. And @MarshallOfSound in over in NZ (god knows what time it is over there).

Github's a brilliant thing. How did we ever live without it. ;-)

MarshallOfSound commented 8 years ago

OK, so reading through these insane walls of text (It's actually great to get so much collab so fast) :smile_cat: I can see that their seems to be some agreement on a few key points, namely:

Fragmented Code

The general idea here appears to be to have one core repository for the current google-music.js library, with other module? repositories that when installed will extend on the core library. Current suggestions for these modular repositories are last.fm intergration and some kind of theming library.

Common Ownership

Here most people seemed concerned with the concept of "cowboy committing", which I too see as a horrendously bad habit to get into. The common ownership should be more of a response time thing, only two people are required to get a fix into master (PR and reviewer),

Versioning / Updating

I don't think I was quite clear on what I meant about independent releases in my original post :disappointed: But it appears as though you picked up on more or less what I was hoping for anyway :grinning:

Basically the branch idea that @jacobwgillespie talks about it what I would be after, being able to load in the latest dist file from a 3.X.X branch that was tagged with stable for instance would be the perfect workflow for me.

What version and how you fetch it would be entirely up the app though, you might decide to push out static files with each release, or to live on the wild side and fetch straight from master (plz no)....

TLDR

We seem to be on more or less the same page, from here I think we should make a new org and move google-music.js their, I've almost got a POC on how to make google-music.js extensible and I'll link that here when it's ready.
We do however need very strict contributing rules for this org as the "bus factor" @jacobwgillespie mentions works both ways, although it might get a group of people to a place faster... If it crashes we all go down with it so bad code in master has got to be prevented at all costs :+1:

chrismou commented 8 years ago

I'd never heard the phrase "bus factor" before, but I like it. Am definitely going to start dropping that into buzzword bingo in meetings at work :laughing:

For the most part, as long as we maintain a decent test suite and don't fall into the trap of :+1:ing potentially breaking changes by eye because they sort of look OK, it should be fine.

Plus once the libraries start getting integrated into the apps we'll at least have several real world examples to test changes against before they get the green light. The feasibility of that should just naturally increase over time.

Anyways, it's 4am over here so I'm off to bed. @MarshallOfSound should be waking up soon... :wink:

MarshallOfSound commented 8 years ago

@chrismou Haven't even had dinner yet mate :P Only 5:30pm

chrismou commented 8 years ago

Oh right, was half asleep. Didn't realise the message before was from you! Wooops.

Now I need to check over all the code I wrote before bed. My brain clearly wasn't operating at full capacity. :-P

twolfson commented 8 years ago

Alright, since we are all on the same page, it's time to start taking action on these pieces:

twolfson commented 8 years ago

Addendum to the GitHub organization: Since everyone has different interests with respect to theming, I would suggest we set up different teams with different admins for each of these repos. Admins on each team can create their own repos (e.g. multiple theming ones -- prob one for the integration, maybe another one that is a collection of CSS variations).

twolfson commented 8 years ago

Damn, hit a snag with rawgit.com. I could have sworn that the cache eventually expired but it turns out it doesn't:

http://rawgit.com/faq#cdn-ttl

I'm sure that if we use the non-CDN version, then we will eventually have too many users and piss people off. There are options like S3 and such but I feel like we are starting to make a mountain out of a molehill =/

MarshallOfSound commented 8 years ago

@twolfson The github API allows 60 unauthorised requests per hour from a given IP address, surely we won't exceed that many requests just fetching a raw file from github?

twolfson commented 8 years ago

This isn't attached to the GitHub API, it's their raw server. If you are unfamiliar with hotlinking to the GitHub raw server, GitHub dislikes it so much that they moved to always sending Content-Type: text/plain to avoid it. As a result, people built things like rawgit.com:

http://rawgit.com/faq#why

We could try using rawgit.com's non-CDN feature but I would recommend against it =/

http://rawgit.com/faq#what-happens-to-jerks

MarshallOfSound commented 8 years ago

This service seems to fulfill the requirements? https://github.com/schme16/gitcdn.xyz/blob/master/README.md

On Mon, 14 Dec 2015, 13:34 Todd Wolfson notifications@github.com wrote:

This isn't attached to the GitHub API, it's their raw server. If you are unfamiliar with hotlinking to the GitHub raw server, GitHub dislikes it so much that they moved to always sending Content-Type: text/plain to avoid it. As a result, people built things like rawgit.com:

http://rawgit.com/faq#why

We could try using rawgit.com's non-CDN feature but I would recommend against it =/

http://rawgit.com/faq#what-happens-to-jerks

— Reply to this email directly or view it on GitHub https://github.com/twolfson/google-music.js/issues/13#issuecomment-164314617 .

twolfson commented 8 years ago

Ah, yea. That should work great. Nice find :+1:

jacobwgillespie commented 8 years ago

I'd put in a vote for cdnjs as the CDN - they have auto-update with npm, which is nice.

twolfson commented 8 years ago

Could you link us to the docs on that?

jacobwgillespie commented 8 years ago

Yep, it's in the README at https://github.com/cdnjs/cdnjs

twolfson commented 8 years ago

Sooo the README says it has a 30 hour turnaround time which prob isn't what we want =/

https://github.com/cdnjs/cdnjs/tree/91057ed60af50889bfc710e70e760b680c5bcaf8#enabling-npmrecommended-or-git-auto-update

Additionally, I don't see anything about dynamic servers (e.g. 2.x.x). It seems to be only for git tag which should be reserved for exact semvers (e.g. 2.3.4)

jacobwgillespie commented 8 years ago

Check out jsDelivr then - looks kinda cool and has some features like version aliasing (which we could use for the 2.x.x versions): https://www.jsdelivr.com/features/jsdelivr-cdn-features

twolfson commented 8 years ago

Yea, that should work as well. The README says it operates based on branches:

https://github.com/jsdelivr/jsdelivr/tree/4cc2747e3164725b672005e4f8b0c37f85f7e253#version-aliasing

So at the end of the day, gitcdn.xyz and jsDelivr would work identically but jsDelivr requires a little bit more setup.

One issue I am having difficulty finding info on is how long jsDelivr caches info for. gitcdn.xyz had 2 hours which is more/less a good TTL for us (prob 5 min to 2 hours is our sweet spot).

jacobwgillespie commented 8 years ago

Yeah, I would vote that we use something like CDNjs or jsDelivr due to the corporate backing - i.e. it's more likely to stay around. I think CDNjs is sponsored primarily by CloudFlare, and jsDelivr is sponsored by several CDNs (MaxCDN, CloudFlare, others). gitcdn.xyz has a note at https://github.com/schme16/gitcdn.xyz/blob/7e2412a05b96389ea78d455203080a3e81dd49ae/README.md#acknowledgments--background saying "As this service is free, I make no guarantees about Up-time nor do I have anything resembling an SLA." Seems more like a hobby project to me.

For jsDelivr, here's an issue that provides some insight (and it looks like it might be possible to get access to a "purge" API that's still in development): jsdelivr/jsdelivr#6913

twolfson commented 8 years ago

From the linked jsdelivr/jsdelivr#6913, there is an issue https://github.com/jsdelivr/jsdelivr/issues/5056 which has a maintainer stating the TTL for aliases is 7 days =/

https://github.com/jsdelivr/jsdelivr/issues/5056#issuecomment-108898293

If we want to use jsDelivr, we will definitely have to get access to their purge implementation so we can cache bust upon deployment.

My recommendation for now would to be move forward with gitcdn.xyz (even though its a bit experimental) but it will move us along to a better place (e.g. have someone to proof of concept with for semver branches for those that want to use them). Then, on the side we can open an issue and see if jsDelivr will grant us access to their purge system.

MarshallOfSound commented 8 years ago

hey guys,

So I managed to get us an open source license to use browserstacks systems to CI this library (woo hoo), if you guys want to be added to the team that has access to the browserstack API just send me a message in the gitter room I added you to, or my email address.

/cc @twolfson @jacobwgillespie @chrismou

chrismou commented 8 years ago

Yo, but late to the party here, but re: org names, would it make more sense to use something a bit more generic? Google Music Utilities? Google Music Library/Libraries?

Especially since there's a good chance the repos won't be 100% JS. Plus I know the pain of working for a company that named after a language - despite having "Flash" in our company's name, we're now about 6 months off dropping Flash support completely in favour of HTML5. Wooops. ;-)

jacobwgillespie commented 8 years ago

I'm actually wondering if it should be named something else that doesn't contain "Google Play" in the name - if our library gets popular, I'm envisioning an email from the Google trademark team... (I believe Radiant went through something of that nature in the past)

chrismou commented 8 years ago

Using a name like "gmusic" is a simple way of getting round that. I assume that's how there's apps like GMaps exist in app stores.

twolfson commented 8 years ago

I'm not sure we should ever have anything non-JS. If we are building something that is a non-JS lib then chances are it could be generalized to not be specific to Google Music (e.g. could be reused for Spotify, YouTube). At that point, I'm not sure it would fit into the organization.

With respect to gmusic, someone is squatting on that in npm so we would have to use something like gmusic.js =/

https://www.npmjs.com/package/gmusic

https://www.npmjs.com/package/gmusic.js

chrismou commented 8 years ago

@twolfson Wasn't it you that mentioned theme repos? That's going to be pretty much exclusively CSS I would have thought. And they'd be specific to overriding styles on the google music stylesheet. They could be used on literally any Google music project, not exclusively JS based.

twolfson commented 8 years ago

Ah, good point :+1:

twolfson commented 8 years ago

The conversation seems to have halted. I'm going to break it down into 2 issues --

MarshallOfSound commented 8 years ago

RE: new name Google Music Framework

has my vote

twolfson commented 8 years ago

I have create a new issue for the floating major/minor releases in #22

twolfson commented 8 years ago

I agree with @chrismou's concerns about trademark infringement. On another project, we had to deal with the painful process as well (e.g. tight deadline requested by company, need to update all docs, update all CI). https://github.com/plaidchat/plaidchat/issues/79

As a result, my vote is for gmusic

chrismou commented 8 years ago

:+1: for gmusic, the org name is already take on GH. gmusic tools? gmusic library? gmusic framework?

Then repo names could be gmusic.js, etc

twolfson commented 8 years ago

Ah, damn =(

How about gmusic-utils?

I don't think library nor framework is exactly fitting for the organization since it's more of an aggregation than an overarching construction.

chrismou commented 8 years ago

Either gmusic-utils or gmusic-tools works for me. Would probably sway towards tools at a push because it's easier to say!

twolfson commented 8 years ago

Heh, a similar rationalization for me about gmusic-utils is that "utils" is more prevalent (e.g. suitcss' utils, GNU's coreutils)

https://github.com/suitcss?utf8=%E2%9C%93&query=utils

http://www.gnu.org/software/coreutils/coreutils.html

MarshallOfSound commented 8 years ago

Leaning towards utils myself :+1:

gmusic-utils

twolfson commented 8 years ago

Alright, I will wait until 12PM CST on Sunday for @jacobwgillespie for submit his vote.

twolfson commented 8 years ago

In any event, here's a checklist derived from what we did on plaidchat:

jacobwgillespie commented 8 years ago

I vote gmusic-utils

twolfson commented 8 years ago

Woot, I will take a shot at performing the rename some time this weekend =)

twolfson commented 8 years ago

Going to grab/squat on that organization ASAP now though ;)

twolfson commented 8 years ago

Alright, organization created and sent out invites :tada: https://github.com/gmusic-utils

twolfson commented 8 years ago

Heh, wrong link from another conversation. Fixed now

jacobwgillespie commented 8 years ago

Was like "already got lunch, but..." :)

:+1:

twolfson commented 8 years ago

Alright, going to perform rename now. We didn't discuss casing at all but here are my thoughts:

twolfson commented 8 years ago

@MarshallOfSound There's a task on the checklist that's relevant to you. Can you update the name of the BrowserStack account? IIRC, it was something like "Google Music...". It should probably be gmusic-utils or gmusic.js now

twolfson commented 8 years ago

I'm going to go create a meta repository now so we can discuss meta topics like avatars for the organization