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

Remove Cargo.lock from gitignore #150

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

torkleyy
Copy link

I think it should be version controlled since it's a binary project, not a library.

@ivmarkov
Copy link
Collaborator

Yes I've heard about this rule before but I have mixed feelings from following it. IMO it results in more maintenance than gains - as users have to issue cargo update right after the generated template to upgrade to latest versions of the crates.

@SergioGasquez @MabezDev @Vollbrecht thoughts?

@torkleyy
Copy link
Author

That's not true unless you add Cargo.lock to this repo, right?

@torkleyy
Copy link
Author

The intention wasn't to lock versions in this repository - since this is not really a binary project, but a template for one. The goal is to achieve full reproducibility for the projects generated from this template, which can only be achieved if they commit the exact versions.

@ivmarkov
Copy link
Collaborator

OK. I would still defer to the other committers for opinion. If they are for it - I'm fine with removing this one line - esp-template - for one - does not have it indeed.

@Vollbrecht
Copy link
Collaborator

I have no strong opinion on that. The only concern is when updating the template that PR's should not accidentally contain a Cargo.lock file in it. Other than that i am fine with it.

@torkleyy
Copy link
Author

I think putting Cargo.lock into the gitignore at the repo root (outside of the template) would prevent that.

@Vollbrecht
Copy link
Collaborator

Here is some interesting new meta information about the topic
https://blog.rust-lang.org/2023/08/29/committing-lockfiles.html

@SergioGasquez
Copy link
Member

Same for me, I don't have any hard opinion on this. But since it has been proposed I am happy to merge it and, if in the future we change or minds we can remove it.

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.

4 participants