aquelemiguel / parrot

🦜 A hassle-free, highly performant, self-hosted Discord music bot with YouTube and Spotify support. Powered by yt-dlp.
MIT License
136 stars 44 forks source link

chore: fix versions in cargo.toml #180

Closed afonsojramos closed 2 years ago

afonsojramos commented 2 years ago

We should ALWAYS lock patch versions in cargo.toml.

Unless you can make sure that all the libraries that we depend on follow semver and that they won't introduce new features in a patch (spoiler alert, you can't - and we've actually done it more than once on our package because you don't want to bump the minor), then you should always lock the versions that you are using.

Please read more about this here or in another post related to this problem in any other language.

afonsojramos commented 2 years ago

@aquelemiguel I recommend that you do some reading with problems related to not fixing versions on a cargo.toml/package.json etc.

joao-conde commented 2 years ago

I mean, if people don't respect semver and introduce breaking changes with patches... then sure, by all means, lock it. But what a bunch of savages.

aquelemiguel commented 2 years ago

I find it really weird locking serde to 1.0.79 for instance because if it adheres to SemVer there's no point, they're bug fixes, we always want more bug fixes... But as you said, @afonsojramos, we're also guilty of introducing small improvements with patches and not everyone follows SemVer. I changed my mind on this.

joao-conde commented 2 years ago

By using: https://crates.io/crates/dependency-refresh

We can do stuff like:

cargo install dependency-refresh
dr -e Cargo.toml

Which will print out new versions and edit the Cargo TOML. I don't see a "check" only option instead of "fixing the TOML" because otherwise could be explored for CI maybe, to alert us when to update.

afonsojramos commented 2 years ago

Which is why the deps.rs project is being widely used across the Rust community. Because it will let you know when you have updates to your dependencies!

joao-conde commented 2 years ago

I find it really weird locking serde to 1.0.79 for instance because if it adheres to SemVer there's no point, they're bug fixes, we always want more bug fixes... But as you said, @afonsojramos, we're also guilty of introducing small improvements with patches and not everyone follows SemVer. I changed my mind on this.

That is why I was always in favor of releasing versions correctly and not to make pretty release numbers 😆

afonsojramos commented 2 years ago

I always pushed for us to release a new minor and not a new patch :eyes:

aquelemiguel commented 2 years ago

Eh, live and learn.

joao-conde commented 2 years ago

I always pushed for us to release a new minor and not a new patch 👀

That is not correct either, you release what you have to release. If it was a patch, release a patch, if it is not one, release a minor. Just respect SemVer or at least don't break it. For example, releasing a patch with breaking changes that should be major is obviously worse than releasing a major version that has only patches or minor stuff because it won't break anything.

afonsojramos commented 2 years ago

I was referring to the 1.2.1, sorry if I was not clear