Moon-0xff / gnome-mpris-label

A music related GNOME extension.
GNU General Public License v3.0
56 stars 11 forks source link

Bug with missing `identity` (Spotube fix) #86

Closed Batwam closed 1 year ago

Batwam commented 1 year ago

I recently discovered Spotube which is an open source which allows to listen to Spotify (even without a premium account). I tried Spotube v3.1.2 and noticed that it didn't have an identity field defined. This has now been reported and fixed in V3.2.0: https://github.com/KRTirtho/spotube/issues/811

While this is more of a bug on the side of the app, this allowed me to notice a couple of this which weren't working when this identity field is missing although they could still work:

  1. the album cover wasn't showing: I have updated the code so the album still appears even when identity isn't defined as this should be required.
  2. the volume adjustment wasn't working: I have updated the code to fallback on System Sound rather than crashing

I also took the opportunity to update the version number and update stream_name to streamName for consistency with the rest of the code.

Moon-0xff commented 1 year ago

Weirdly enough, we benefit from broken mpris interfaces! 😄

Regarding the versioning I think only the patch should be bumped, the most recent version of this extension for 43/44 still is 25.

Batwam commented 1 year ago

Yeah, I was thinking about that. Shouldn't we upload both versions in parallel so they both have the improvements? Or is the gnome43/44 version going to stay frozen at v25? I assumed that the whole point of the patch was to continue developing both in parallel.

Moon-0xff commented 1 year ago

Of course, but v26 doesn't have improvements over v25. At least not from the user perspective.

I'll update the version when there's something to write on the changelog. I don't like to push (or receive) small updates, specially when they don't include a changelog!

Batwam commented 1 year ago

Yeah, we don't need to push this just yet and perhaps realign the version next time you need to upload. Are you allowed to skip a version?

Moon-0xff commented 1 year ago

I don't set the versions.

As I understand, the multi-versioning in ego is just how it works normally. It sets the latest version of the extension that matches the gnome versions specified in metadata.json.

So if I upload (for example) a version of the extension for 3.36 it will bump the version for 3.36, the rest is unchanged.

This will make the versioning numbers a fuzz, but if I time the updates correctly I hope it will not look so bad.

Moon-0xff commented 1 year ago

Look! I found an example.

Caffeine latest version is for GNOME 40. Very anachronistic!

Batwam commented 1 year ago

I see what you mean. In this case yes, we will probably end up with "out of sync" version between the gnome 43/44 and gnome 45 versions but that's not the end of the world. I'll update the version numbers based on current+1 for each version.

Moon-0xff commented 1 year ago

I'll update the version numbers based on current+1 for each version.

That would skip an update.

Batwam commented 1 year ago

stable is on EGO is currently v25 for gnome43/44 and v26 for gnome 45 so main should be one up from this?

Moon-0xff commented 1 year ago

I now realize, the updater would work fine with this (I think). But this is more of a mess of numbers!

It's better if we simply follow whatever ego assigns.

Batwam commented 1 year ago

Ok so, we use for main the same numbers as current stable and only update once we have the final EGO numbers assigned? I don't mind using the same numbers but it feels like using +1 for main makes sense to indicate that it's a higher version. If the EGO review forces to increase after submission we can still do that but it's never going to be less than what we have assumed anyway.

In short, let me know if you want to use 25/26 to align with EGO stable or use 26/27 as proposed. Either way, the gnome 45 version needs to be updated to at least 26 as it's currently one version behind EGO and it gets overwritten by the extension managers.