Check in Cargo.lock - #2482
Conversation
LucaCappelletti94
left a comment
There was a problem hiding this comment.
To make dependency updates reliably visible in review, maybe we should use --locked in the CI? The sqlparser_bench job uses an independent manifest, so it likely needs its own committed lockfile, or possibly better a workspace integration.
| @@ -0,0 +1,412 @@ | |||
| # This file is automatically @generated by Cargo. | |||
| # It is not intended for manual editing. | |||
| version = 4 | |||
There was a problem hiding this comment.
At this time https://github.com/apache/datafusion-sqlparser-rs/blob/main/.tool-versions still specifies Rust 1.75, and version = 4 is only supported from 1.78. I suspect that the current CI misses this because it tests only stable, beta, and nightly, and not specifically 1.75.
I am not sure whether here we would want to either bump .tool-versions or downgrade the version of the Cargo.lock.
|
thanks @LucaCappelletti94 -- I defer to your judgement for such changes. Maybe we should just punt on thsi PR until we have some issue due to dependencies 🤷 |
I had to Google what |
|
Sounds good -- thank you |
Which issue does this PR close?
N/A
Rationale for this change
Checking in
Cargo.lockgives CI reproducible builds and makes dependency updates visible in review, per the current Cargo guidance which now recommends committing the lockfile for libraries as well.What changes are included in this PR?
Cargo.lockfrom.gitignoreCargo.lockgenerated withcargo update(all 46 packages locked to the latest semver-compatible versions; noCargo.tomlchanges needed)Are these changes tested?
cargo test --all-featurespasses (22/22 suites, 0 failures).Are there any user-facing changes?
No. This only affects local development and CI of this repository; downstream users of the published crate are unaffected.