mozilla / fxa-email-service

DEPRECATED - Migrated to https://github.com/mozilla/fxa
Mozilla Public License 2.0
6 stars 6 forks source link

Specify dependency versions as ">x.y.z" in Cargo.toml #40

Closed philbooth closed 6 years ago

philbooth commented 6 years ago

The workflow for updating dependencies when you specify full version strings in Cargo.toml is more awkward and doesn't actually add any benefits (imho).

This is my workflow for updating dependencies:

If all of the version strings in Cargo.toml are "*" ">x.y.z", we'll automatically take the last version of everything then test stuff still works. If there are failures, we can resolve them on a case-by-case basis.

However, if we use version strings in Cargo.toml, it only updates as far as it can without changing the most-significant (non-zero) component of the version string (docs). To update beyond that requires doing a manual cargo search foo or whatever for each dependency and then changing the version in Cargo.toml accordingly if a newer version is available. All of the steps for testing and so on are the same, it just makes the update part more difficult.

Hence I'm proposing we specify "*" ">x.y.z" for everything and make life easier. (we could also use the > operator but I don't see the point really)

shane-tomlinson commented 6 years ago

Would you want to go back to specific versions once this ships to prod? IIRC, the reason we pin to versions is so that we can remove one variable, dependency version, if tests fail on stage/prod but not locally.

vladikoff commented 6 years ago

we could also use the > operator but I don't see the point really

I like those instead of *

vladikoff commented 6 years ago

If we do rocket = ">0.3.12" and at some point Rocket breaks, we can just set `rocket = ">0.3.12, <1" and call it a day, I think it is more well defined that way. Otherwise the "*" makes it harder to 'bisect' a broken dependency

philbooth commented 6 years ago

Would you want to go back to specific versions once this ships to prod?

Not really.

IIRC, the reason we pin to versions is so that we can remove one variable, dependency version, if tests fail on stage/prod but not locally.

We still get that without putting hard version strings in Cargo.toml fwiw, because Cargo.lock always hard-codes a version string regardless. All this affects is the behaviour when we run cargo update.

philbooth commented 6 years ago

If we do rocket = ">0.3.12" and at some point Rocket breaks, we can just set `rocket = ">0.3.12, <1" and call it a day, I think it is more well defined that way. Otherwise the "*" makes it harder to 'bisect' a broken dependency

Good point, well made. I am persuaded.

brizental commented 6 years ago

Hey guys, I will work on this :)