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

Stop ignoring Cargo.lock #441

Closed
wants to merge 1 commit into from
Closed

Conversation

DJMcNab
Copy link
Member

@DJMcNab DJMcNab commented Feb 15, 2024

This follows the guidance in https://blog.rust-lang.org/2023/08/29/committing-lockfiles.html

Previous discussion in #256

I've opened this as a draft, because it looks like it requires more CI work to ensure this is updated regularly.
I don't know that we want to commit to developing that

Copy link
Collaborator

@waywardmonkeys waywardmonkeys left a comment

Choose a reason for hiding this comment

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

I don't feel super strongly about this because I can (and have) argued both sides.

I suspect that if this does happen (or even if it doesn't), we should add CI for "minimal versions".

@DJMcNab
Copy link
Member Author

DJMcNab commented Feb 15, 2024

To explain the history, I started making this change, then read the linked document chain, and realised it required some more care, and the advantages might not match the amount of effort needed

@xStrom
Copy link
Member

xStrom commented Feb 15, 2024

I've been doing some research into it and have a lot more to say about this (and MSRV) and will post a comprehensive explanation in not too distant of a future, hopefully before the end of the month. Until then the short of it is that it's very messy and complicated.

We currently have Cargo.lock committed for Xilem, so in that sense not a big issue to add it here for now. Once I post my findings, hopefully we can settle on a unified strategy across repos.

@xStrom
Copy link
Member

xStrom commented Mar 1, 2024

I did get to posting my comprehensive explanation. It is available for reading at rfcs#5.

@DJMcNab
Copy link
Member Author

DJMcNab commented Mar 1, 2024

It seems like we'll have a more standardised experience, so I'll close this PR

@DJMcNab DJMcNab closed this Mar 1, 2024
@DJMcNab DJMcNab deleted the lock-ignore branch March 1, 2024 09:27
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