Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: fix versions in cargo.toml #180

Merged
merged 2 commits into from
Mar 30, 2022
Merged

chore: fix versions in cargo.toml #180

merged 2 commits into from
Mar 30, 2022

Conversation

afonsojramos
Copy link
Collaborator

@afonsojramos afonsojramos commented Mar 30, 2022

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.

Copy link
Owner

@aquelemiguel aquelemiguel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @joao-conde said in the other PR, I also don't think it makes sense to lock deps to patch versions so that we can have a badge in the README.

@afonsojramos
Copy link
Collaborator Author

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

@joao-conde
Copy link
Collaborator

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
Copy link
Owner

aquelemiguel commented Mar 30, 2022

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
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Collaborator

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
Copy link
Collaborator Author

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

@aquelemiguel
Copy link
Owner

Eh, live and learn.

@joao-conde
Copy link
Collaborator

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
Copy link
Collaborator Author

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

@joao-conde joao-conde merged commit 0cf5770 into main Mar 30, 2022
@joao-conde joao-conde deleted the fix-toml branch March 30, 2022 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants