Skip to content

Conversation

@QIU1995NONAME
Copy link
Contributor

  • I agree to follow the project's code of conduct.
  • I added an entry to rstar/CHANGELOG.md if knowledge of this change could be valuable to users.

Copy link
Member

@michaelkirk michaelkirk left a comment

Choose a reason for hiding this comment

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

I'm personally a fan of including the Cargo.lock, mostly because it makes it easier to build historic commits when you need to do ancient code archeology. It does also remove unexpected drama from PR's (like #211).

The main downside I see, is by pinning old versions in a lockfile, CI might hide build problems that real users today are having.

On balance, I still think we should merge, but would want others to weigh in.


The immediate failure was:

error: package syn v2.0.109 cannot be built because it requires rustc 1.68 or newer, while the currently active rustc version is 1.63.0

So instead of, or in addition to, committing the Cargo.lock, we could raise our MSRV.

@michaelkirk
Copy link
Member

Maybe also related/solved by https://github.com/georust/rstar/pull/204/files ?

@adamreichold
Copy link
Member

AFAIK, checking in Cargo.lock is the new recommendation, c.f. https://doc.rust-lang.org/cargo/faq.html#why-have-cargolock-in-version-control. We could have a CI job that builds after deleting that file if we fear breakage from newer versions.

@michaelkirk michaelkirk added this pull request to the merge queue Nov 7, 2025
Merged via the queue into georust:master with commit 5224382 Nov 7, 2025
6 checks passed
@QIU1995NONAME QIU1995NONAME deleted the restore-cargo-lock branch November 9, 2025 02:23
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.

3 participants