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

feat: add frozen and locked #198

Merged
merged 4 commits into from
Jul 26, 2023
Merged

Conversation

jayy-lmao
Copy link
Contributor

Issue #, if available:

Description of changes:
This introduces the frozen and locked behaviors to cargo chef.
See: https://doc.rust-lang.org/cargo/commands/cargo-build.html#manifest-options

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

src/main.rs Outdated Show resolved Hide resolved
@atwam
Copy link

atwam commented Jul 18, 2023

Hi, just saw this PR as I was about to work on something similar, this time to pass on --verbose to the underlying cargo calls.
@jayy-lmao, if you mind including verbose the same way you have frozen and locked when you fix the comments as per @LukeMathWalker request, that would be great.
If not, no big deal, I'll take inspiration in your PR to submit a PR of my own.

@jayy-lmao
Copy link
Contributor Author

Hi, just saw this PR as I was about to work on something similar, this time to pass on --verbose to the underlying cargo calls. @jayy-lmao, if you mind including verbose the same way you have frozen and locked when you fix the comments as per @LukeMathWalker request, that would be great. If not, no big deal, I'll take inspiration in your PR to submit a PR of my own.

Just did this now, ty

src/main.rs Outdated Show resolved Hide resolved
@vidhanio
Copy link
Contributor

with the addition of these flags, would --frozen become the "default" way to use cargo-chef now? i don't see why someone wouldn't want their ci builds to be frozen and only read from Cargo.lock.

@LukeMathWalker LukeMathWalker merged commit 0934944 into LukeMathWalker:main Jul 26, 2023
4 checks passed
@LukeMathWalker
Copy link
Owner

It depends—generally speaking, it would probably be alright to make --frozen the default, but it's likely to break some builds and I don't think the benefits outweight the risks here.

@vidhanio
Copy link
Contributor

@LukeMathWalker i meant default more as in "recommended" configuration, like adding it to the README.

@LukeMathWalker
Copy link
Owner

I don't think it adds enough value and it would probably lead to people using it without understanding what it actually does (because they are copying from the example), which would in turn lead to open issues here.

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