Skip to content

Conversation

@mk-rw
Copy link
Contributor

@mk-rw mk-rw commented Nov 25, 2025

Due to the splintered nature of dependencies in the workspace, it can be difficult to perform monorepo-wide updates of dependencies, which can create subtle bugs due to incompatible dependency versions. Additionally, some rather obscure bugs can occur with some dependencies (e.g. defmt, embassy-executor) due to linker behaviour.

Thus, this PR aims to consolidate most dependencies which may be reused often in the global Cargo.toml which greatly simplifies not only updating dependencies, but also setting up new crates in the project (e.g. crates used on the boards often want dependencies with defmt compatibility, so we can enable this feature at the workspace level).

Finally, it may be beneficial to include
cargo-deny for granular control over dependencies, versioning and other cargo stuff (e.g., warning the user when a workspace-level dependency is duplicated)

Due to the splintered nature of dependencies in the
workspace, it can be difficult to perform monorepo-wide updates of
dependencies, which can create subtle bugs due to incompatible
dependency versions. Additionally, some rather obscure bugs can occur
with some dependencies (e.g. defmt, embassy-executor) due to linker
behaviour.

Thus, this branch aims to consolidate most dependencies which may be
reused often in the global `Cargo.toml` which greatly simplifies not
only updating dependencies, but also setting up new crates in the
project (e.g. crates used on the boards often want dependencies with
`defmt` compatibility, so we can enable this feature at the workspace
level).

Finally, it may be beneficial to include
[`cargo-deny`](https://github.com/EmbarkStudios/cargo-deny) for granular
control over dependencies, versioning and other cargo stuff (e.g.,
warning the user when a workspace-level dependency is duplicated)
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR consolidates workspace dependencies into the root Cargo.toml to simplify dependency management and prevent version conflicts across the monorepo. The changes enable defmt compatibility features at the workspace level for embassy crates.

  • Centralized 20+ commonly-used dependencies in workspace-level configuration with defmt features enabled by default
  • Updated boards/argus to inherit workspace dependencies, removing redundant version and feature specifications
  • Alphabetized dependencies in apps/sergw for better organization

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
Cargo.toml Added workspace-level dependency definitions with defmt feature flags enabled for embassy crates and other common dependencies
boards/argus/Cargo.toml Migrated to workspace dependencies, inheriting versions and base features while keeping board-specific features local
apps/sergw/Cargo.toml Reordered dependencies alphabetically
Cargo.lock Updated to reflect new dependency resolutions, including embassy-futures now directly depending on defmt 1.0.1

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Cargo.toml Outdated
cortex-m = { version = "0.7.6", features = ["critical-section-single-core"] }
cortex-m-rt = "0.7.1"
critical-section = "1.1"
csv = { path = './common/csv' }
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

Inconsistent quote style: the path for csv uses single quotes ('./common/csv') while the path for messages on line 35 uses double quotes ("./common/messages"). For consistency within the workspace dependencies, both should use the same quote style. Recommend using double quotes for both.

Suggested change
csv = { path = './common/csv' }
csv = { path = "./common/csv" }

Copilot uses AI. Check for mistakes.
@darrenrahnemoon
Copy link
Member

darrenrahnemoon commented Nov 28, 2025

Good idea on the cargo deny! Wanna hook it up as a GH action so that in all PRs it'll check for that? Could be a part of one of the existing workflows. Could be a part of the lint job and we rename lint to sanity_checks or maybe a separate job in .github/workflows/argus-build.yml
We could also really benefit from a phoenix-build in a similar way to make sure the phoenix build passes before every PR merge that could be in a separate PR if you feel like it would take longer to setup

This will allow us to catch a lot of dependency-based issues ahead of
time, like vulnerabilities, maintenance, duplication, sources. For
example, this will allow us to catch dependencies that should be using
the global level dependency, but instead uses its own and (potentially)
creates version conflicts and duplication of dependencies.
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