Skip to content
This repository has been archived by the owner on Mar 26, 2021. It is now read-only.

chore: remove Cargo.lock #65

Merged
merged 1 commit into from
Mar 5, 2021
Merged

Conversation

joshuajbouw
Copy link
Member

@joshuajbouw joshuajbouw commented Feb 23, 2021

The Cargo.lock was left in for the library. This is wrong and should be removed as you may run into confusing situations as #63 highlights. Typically, it is only acceptable to have a Cargo.lock file for binaries when you need the compiled code to be exactly the same on everyone's machines. For libraries, this simply isn't required and highly not recommended.

A bit more reading, a Cargo.locks point is to lock the version state of all dependencies to ensure that the builds across all devices is the same with no surprises as of course, who knows what may pop up in a new patch of a dependency. Leaving it in for a library the dependencies will be checked by binaries / other libraries that use the library and will result in dependency clashes.

Rule of thumb: If its a binary as it is the end of a dependency chain, keep the Cargo.lock file. If it is a library, do not include it.

This will not automatically solve the problem if you already have a Cargo.lock, you MUST run cargo update to fix this post this PR. It will prevent others from getting into trouble with it though.

Reference: https://doc.rust-lang.org/cargo/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries

@artob
Copy link
Contributor

artob commented Feb 25, 2021

@ilblackdragon Any objection to merging this?

@artob artob self-assigned this Feb 25, 2021
@@ -5,3 +5,4 @@ neardev/
src/test/build/
src/tests/build/
src/tests/node_modules
Cargo.lock
Copy link
Contributor

Choose a reason for hiding this comment

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

@joshuajbouw In the future, please ensure all text files are terminated by a newline.

@artob artob merged commit c6623fd into near:master Mar 5, 2021
@ilblackdragon
Copy link
Member

ilblackdragon commented Mar 8, 2021 via email

@joshuajbouw
Copy link
Member Author

You are correct, now that I'm aware of how we are using WASM as it is indeed compiled. This is important.

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants