spotifyd icon indicating copy to clipboard operation
spotifyd copied to clipboard

fix inital_volume being a string

Open JojiiOfficial opened this issue 4 years ago • 13 comments

JojiiOfficial avatar Jan 16 '21 18:01 JojiiOfficial

unfortunately this is a breaking change, we could also change the readme to initial_volume = "90" and keep the weird beheviour but that isnt great either

robinvd avatar Jan 16 '21 18:01 robinvd

That is true but we don't need to publish a release instantly. Most people will use the latest stable version and this new way of parsing the "initial_value" will probably come with other minor breaking changes in the next version.

JojiiOfficial avatar Jan 16 '21 18:01 JojiiOfficial

yes that might be a good plan, the only thing just like the TOML pr is that people will copy the config from the readme, and that wont be compatible

robinvd avatar Jan 16 '21 19:01 robinvd

I added this to the next breaking change milestone.

We should probably decide if we want to keep the incorrect documentation until then.

slondr avatar Jan 17 '21 02:01 slondr

We should update the documentation to work with the currently stable version and update it when we release the new version. We could probably just add a comment that with a git version of spotifyd, the value must be set differently.

JojiiOfficial avatar Jan 17 '21 10:01 JojiiOfficial

Or we could point the default branch to the latest release

robinvd avatar Jan 17 '21 12:01 robinvd

Yes I was thinking about this as well. Usually I'm not a fan of such a solution because it looks like nothing is developed anymore and most people see the master branch as nightly/latest version (as far as I noticed) but in such a case like this, it is probably a solution we should consider.

JojiiOfficial avatar Jan 17 '21 13:01 JojiiOfficial

Or we could just hold off on merging this until we're ready to release 0.4.

slondr avatar Jan 17 '21 15:01 slondr

Ye, just keep the PR open and merge once v0.4 is on the horizon. It's the easiest.

mainrs avatar Jan 17 '21 18:01 mainrs

I noticed this and wanted to bring up changing the default branch to point to v.0.3.0, whereby drive-by readme readers could be shown the readme for the stable release and anyone looking to compile the master branch could do so with a simple git switch master. A note could be added to the readme for anyone who doesn't immediately notice that the default branch points to a stable release.

That way, breaking changes could be included with a consistent readme in master for people who want to build the current development branch, leading to more bleeding-edge users successfully testing the development channel and finding documentation errors and bugs (especially those arising from merging multiple breaking changes all together at release time) before release.

leonm1 avatar Feb 20 '21 16:02 leonm1

Alternatively, do we publish the book anywhere? It would probably make more sense if we had properly versioned documentation without expecting users to realize the cargo.toml is a year out-of-date, lol

slondr avatar Sep 29 '22 00:09 slondr

I'm not sure if you mean that, but the book is published here. It seems, however, that it is updated together with the master branch. Maybe we could

  • either maintain two versions of it: e.g. https://spotifyd.github.io/spotifyd/master and https://spotifyd.github.io/spotifyd/latest
  • or change that to only update on releases

eladyn avatar Sep 30 '22 21:09 eladyn

One could change this to be an enum with String and u16 to smooth out the transition: Warn if the user provided a string but still allow it. This would allow users to smoothly change to u16 for the value and in the next release support for String can get removed.

rnestler avatar Dec 04 '23 20:12 rnestler