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

Cargo.lock file ignored, so builds between machines could use different dependencies #403

Closed
jzaki opened this issue Jun 19, 2019 · 4 comments · Fixed by #684
Closed

Cargo.lock file ignored, so builds between machines could use different dependencies #403

jzaki opened this issue Jun 19, 2019 · 4 comments · Fixed by #684
Assignees
Labels
good first issue Good for newcomers

Comments

@jzaki
Copy link
Contributor

jzaki commented Jun 19, 2019

Description

Cargo.lock shouldn't be ignored for binaries https://doc.rust-lang.org/cargo/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries

Present Behaviour

Dev who runs cargo update after a dependency update will have a different version to existing devs

Expected Behaviour

Guarantee same versions of dependencies between devs

Steps to resolve

Remove Cargo.lock from .gitignore

@jzaki
Copy link
Contributor Author

jzaki commented Jun 19, 2019

This may not be a priority whilst at this stage of development, and everyone runs cargo update on occasion.

@paulhauner paulhauner added this to the v0.0.1 milestone Jun 25, 2019
@paulhauner paulhauner added the good first issue Good for newcomers label Jul 4, 2019
@krsnnik
Copy link
Contributor

krsnnik commented Aug 26, 2019

Hey guys, is this still needed? I can definitely remove Cargo.lock from .gitignore if needed at this time.

@paulhauner
Copy link
Member

Hey @krsnnik, thanks for reaching out!

I haven't thought about this one in a while. It's in fact adding the Cargo.lock file to the repo :)

I would love your help, but my feeling is that we should have a "core developer" (someone with write access to this repo) do this one. It would generate a rather large file that would be difficult to audit, exposing an opportunity to inject a bad dep.

I have no doubt you're a venerable, upstanding eth-citizen! I just think it would be best practice this way :)

If you're in the spirit of contributing, I just made this simple one as an alternative: #512

@paulhauner paulhauner removed this from the v0.0.1 milestone Oct 25, 2019
@paulhauner paulhauner added this to the Public Testnet milestone Nov 21, 2019
@paulhauner
Copy link
Member

paulhauner commented Nov 21, 2019

I will do this sometime before testnet release.

@paulhauner paulhauner self-assigned this Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants