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

Add Cargo.lock for dependency consistency #315

Merged
merged 1 commit into from
May 6, 2024
Merged

Add Cargo.lock for dependency consistency #315

merged 1 commit into from
May 6, 2024

Conversation

bitboom
Copy link
Collaborator

@bitboom bitboom commented May 2, 2024

This commit adds the Cargo.lock file to ensure that the exact versions of dependencies are locked down across all environments.

This is crucial for maintaining consistent behavior of our application in development, testing, and production, reducing the works on my machine issues by aligning the dependency versions used by all developers and in all deployments.

ref) https://blog.rust-lang.org/2023/08/29/committing-lockfiles.html

This commit adds the Cargo.lock file to ensure that
the exact versions of dependencies are locked down
across all environments. This is crucial for maintaining
consistent behavior of our application in development,
testing, and production, reducing the 'works on my machine'
issues by aligning the dependency versions used by all developers
and in all deployments.

ref) https://blog.rust-lang.org/2023/08/29/committing-lockfiles.html

Signed-off-by: Sangwan Kwon <[email protected]>
@bitboom bitboom marked this pull request as ready for review May 3, 2024 00:42
@zpzigi754
Copy link
Collaborator

After reading the reference, I am still not sure whether adding Cargo.lock is a desirable policy in our case, as previously it was recommended not to add Cargo.lock for the libraries (maybe the Cargo's general recommendation can change again).

The change would be summarized as the below. I think that testing with newer dependencies is a good feature which is currently done by CI (and can be reproduced locally with cargo update), but can be gone with the change.

[before this PR]

  • CI will check with the newest versions in the dependencies
  • Cargo.lock doesn't have to be maintained

[after this PR]

  • CI will check with the same old versions in the dependencies
  • Cargo.lock needs to be maintained which can be another burden

If what we want is fixing the versions of certain third-party crates, another option we can use would be modifying Cargo.toml.

@bitboom
Copy link
Collaborator Author

bitboom commented May 3, 2024

After reading the reference, I am still not sure whether adding Cargo.lock is a desirable policy in our case, as previously it was recommended not to add Cargo.lock for the libraries (maybe the Cargo's general recommendation can change again).

The change would be summarized as the below. I think that testing with newer dependencies is a good feature which is currently done by CI (and can be reproduced locally with cargo update), but can be gone with the change.

[before this PR]

  • CI will check with the newest versions in the dependencies
  • Cargo.lock doesn't have to be maintained

[after this PR]

  • CI will check with the same old versions in the dependencies
  • Cargo.lock needs to be maintained which can be another burden

If what we want is fixing the versions of certain third-party crates, another option we can use would be modifying Cargo.toml.

We released the SDK working on the certifier as a tag certifier-v1.0-beta.
However, our code has not changed, due to an update from a third party (zeroize), the previously released version did not work.

Either way, we have to stop this.
Modifying Cargo.toml is good for fixing the version we need, but it cannot solve this problem.

Copy link
Collaborator

@zpzigi754 zpzigi754 left a comment

Choose a reason for hiding this comment

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

If Cargo.lock would help to pinpoint the issue and others, adopting it could be beneficial.

@bitboom bitboom merged commit 992c887 into main May 6, 2024
10 checks passed
@bitboom bitboom deleted the cargo-lock branch May 6, 2024 23:02
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.

2 participants