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

chore: standardise Cargo.toml files #543

Merged
merged 1 commit into from
Aug 16, 2024

Conversation

johnyob
Copy link
Collaborator

@johnyob johnyob commented Aug 15, 2024

Context

Description

This PR

In future,

  • External dependencies must be added to the root Cargo.toml (unless not supported as a workspace dependencies, such as optional dependencies)
  • Internal crates are referred to using { path = <crate path> }

Manually testing the PR

Check everything builds and passes tests :)

make build
make test

This commit standardises all Cargo.toml files.
We use workspace dependencies for *true* workspace dependencies.
Crates within the workspace are referred to using their path.
http.workspace = true
in-container.workspace = true
indicatif.workspace = true
jstz_api = { path = "../jstz_api" }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of curiosity, why can't we refer to internal crates with .workspace?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since we rely on .workspace to differentiate between external dependencies and internal crates.
We rely on this in #542 when building all the workspace dependencies (cargoDeps). If we don't have this distinction, then we'd essentially get cache misses each time, as the workspace dependencies would include all internal crates (which change in almost every PR)

@zcabter
Copy link
Collaborator

zcabter commented Aug 16, 2024

Build and tested locall ✅

@zcabter zcabter assigned johnyob and unassigned zcabter Aug 16, 2024
@johnyob johnyob merged commit 8fb97fb into main Aug 16, 2024
6 checks passed
@johnyob johnyob deleted the ajob410@chore/standardise-cargo-toml branch August 16, 2024 10:05
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