Replace version functions by constants

[Johannesd3]

Since a breaking change is coming anyway, we could also replace the fns in the version module by consts, which is closer to their actual purpose I think. I removed those strings that weren't used, but they could be added anytime. Removing them would be a breaking change.

Before merging it, the following questions should be answered.

1. Are the names and the documentation ok?
2. Is really every of the used constants necessary?
3. Shouldn't the cargo package version be used somewhere?
4. Is an own module for those five consts really necessary?
5. Are there any dependent crates using one of the strings I removed?

[sashahilton00]

Not sure how one would integrate the cargo package version, without some sort of clunky parsing of the toml file, unless there is some rust function for handling this that I am unaware of? Also, not sure I'd consider this breaking; technically it is, but realistically these methods are only being used internally in librespot, I haven't seen or heard of anyone using them outside of it.

[Johannesd3]

The cargo version was already implemented (fn semver()), but I removed it for now since it wasn't used anywhere. Maybe it would be good to show this version in some version string? Then it's easy to add again.

Spotifyd uses the version mod. But imo every change that is "technically" breaking should be considered as breaking, as we don't know how people use the librespot crate privately. (This was the reason why I removed all unnecessary functions; adding them is no breaking change.)

[sashahilton00]

But imo every change that is "technically" breaking should be considered as breaking, as we don't know how people use the librespot crate privately. (This was the reason why I removed all unnecessary functions; adding them is no breaking change.)

Yeah that's fair enough. Wrt changing from fn to const I think that it makes sense. Though since it's a) a breaking change, and b) we will need to wait and see if there is anyone that voices their usage of any of the strings you removed, we'll look at merging this after 0.2.0 as it's not worth holding up the release for.

With regards to the semver stuff you mentioned, it would be good to show the current version in the startup string as in many cases the commit sha isn't all that useful alone. possibly a format such as librespot-0.2.0-09e506e-ae189df where the latter two parts are the commit short sha and build id would be better. We could also display the build date afterwards if that's desired.

[ashthespy]

4\. Is an own module for those five `const`s really necessary?

It's a build-dependencies, so it should be fine. The other option is to shell out to git/pull in libgit2/ directly read from .git folder?

With regards to the semver stuff you mentioned, it would be good to show the current version in the startup string as in many cases the commit sha isn't all that useful alone.

FWIW, I went overboard :-D

./vollibrespot -v
vollibrespot v0.2.2 cd96399 2020-10-07 (librespot 08d8bcc 2020-10-07) -- Built On 2020-10-07

[sashahilton00]

Yeah that is a bit overboard :P I think something similar could work though. I haven't checked the librespot startup message recently, but something like librespot v0.2.0 cd96399 (Built on 2021-02-10, Build ID: a789df1) is probably adequate.

[sashahilton00]

I've added in a version string CLI flag in the form of --version or -V, as pretty much every CLI app provides this functionality. I've also made the name parameter optional and default to Librespot, which as a side effect means that commands such as the one to list available backends can now be run as librespot --backend ? as opposed to librespot -n librespot --backend ?

[sashahilton00]

I think we're probably good to go on this.

[Johannesd3]

So do you want to merge this before 0.2? Is there anything at all that blocks 0.2?