Skip to content

Add lock files to git - #14

Closed
mkysel wants to merge 1 commit into
mainfrom
mkysel/lock-files
Closed

mkysel wants to merge 1 commit into
mainfrom
mkysel/lock-files

Conversation

@mkysel

@mkysel mkysel commented Oct 17, 2022

Copy link
Copy Markdown
Contributor

I am not quite sure why we put the lock files into gitignore. This means that dependencies are not really controlled.

@mkysel
mkysel requested review from hwchen and vgel October 17, 2022 14:26
@hwchen

hwchen commented Oct 17, 2022

Copy link
Copy Markdown
Contributor

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

https://users.rust-lang.org/t/is-it-wrong-to-have-a-cargo-lock-in-a-library-crate/59358

Generally rust libraries don't commit lockfiles. I think if there's a specific usecase for it (like the second link) we can do it.

@mkysel

mkysel commented Oct 17, 2022

Copy link
Copy Markdown
Contributor Author

its kinda funny that we have dependabot set up, but don't have a lock file :) If we want to behave like a library, we should probably git rid of the bot.

I would also argue that given that this is a python library, it should be deterministic.

I guess the duality of a rust library vs a python library makes it complicated.

@vgel

vgel commented Oct 17, 2022

Copy link
Copy Markdown
Contributor

Hmm, that's a good point. Probably the best thing to do would be to have two crates, but not in a workspace -- one crate is the rust lib and the other is the python lib, and since they're not in a workspace the python one can have a lockfile while the rust one doesn't?

@vgel

vgel commented Oct 17, 2022

Copy link
Copy Markdown
Contributor

I'm going to close this but open an issue referencing it.

@vgel vgel closed this Oct 17, 2022
@vgel
vgel deleted the mkysel/lock-files branch December 15, 2022 20:12
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