Skip to content

Conversation

@dani-garcia
Copy link
Member

@dani-garcia dani-garcia commented Dec 9, 2025

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-29535

📔 Objective

Upgrades the version of cargo-sort used to the latest. This changes slightly the ordering and formatting of some of the Cargo.toml files, but they should be functionally identical. The change should help reduce the ammount of accidental changes made by devs when they install a too new version of cargo-sort.

If you're running cargo-sort locally, make sure it's updated and that you're running the correct command (Now it needs a --grouped parameter). I'll post this on the SDK channel as well once the change is merged:

# To install it
cargo install cargo-sort --version 2.0.2 --locked

# To sort and format the cargo manifests
cargo sort --workspace --grouped

# To check if the files are sorted correctly, without making any changes
cargo sort --workspace --grouped --check

🚨 Breaking Changes

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation
    team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed
    issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

@github-actions
Copy link
Contributor

github-actions bot commented Dec 9, 2025

Logo
Checkmarx One – Scan Summary & Details0cecea26-2513-4cde-86ea-77c9a97658f4

Fixed Issues (1)

Great job! The following issues were fixed in this Pull Request

Severity Issue Source File / Package
LOW Parameter_Tampering /crates/bitwarden-uniffi/kotlin/app/src/main/java/com/bitwarden/myapplication/MainActivity.kt: 400

@dani-garcia dani-garcia changed the title Update cargo-sort [PM-29535] Update cargo-sort Dec 9, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 9, 2025

🔍 SDK Breaking Change Detection Results

SDK Version: ps/update-cargo-sort (f8d3613)
Completed: 2025-12-09 18:21:32 UTC
Total Time: 218s

Client Status Details
typescript ✅ No breaking changes detected TypeScript compilation passed with new SDK version - View Details

Breaking change detection completed. View SDK workflow

@codecov
Copy link

codecov bot commented Dec 9, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.59%. Comparing base (a8a19d3) to head (f8d3613).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #602   +/-   ##
=======================================
  Coverage   78.59%   78.59%           
=======================================
  Files         281      281           
  Lines       29131    29131           
=======================================
  Hits        22895    22895           
  Misses       6236     6236           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dani-garcia dani-garcia marked this pull request as ready for review December 10, 2025 11:24
Copilot AI review requested due to automatic review settings December 10, 2025 11:24
@dani-garcia dani-garcia requested review from a team as code owners December 10, 2025 11:24
@claude
Copy link

claude bot commented Dec 10, 2025

Claude finished @dani-garcia's task —— View job


Code Review for PR #602

Todo List

  • Analyze PR changes with bitwarden-code-reviewer agent
  • Compile review feedback
  • Post comprehensive review

Overall Assessment: APPROVE 👍

This PR updates cargo-sort from a git-pinned version to the official 2.0.2 release. All changes are purely formatting and ordering related - no functional changes to any dependencies, features, or code logic.

Key Findings

✅ Positive Observations

  1. Consistent cargo-sort update across all touchpoints:

    • .github/workflows/lint.yml:90-93: Updated installation command and added --grouped flag
    • README.md: Updated documentation for developers
    • lint-staged.config.js: Updated pre-commit hook
    • All 24 Cargo.toml files reformatted consistently
  2. No functional changes detected:

    • All dependency versions remain identical
    • Feature flags unchanged
    • No changes to build profiles or compilation settings
    • Only section ordering and formatting modified
  3. CI verification passed:

    • ✅ Breaking change detection: No breaking changes
    • ✅ All lint checks passing
    • ✅ Code coverage unchanged (78.59%)
    • ✅ Checkmarx security scan passed

📝 Formatting Changes Applied

The new cargo-sort 2.0.2 applies these formatting preferences consistently across all Cargo.toml files:

  1. Section reordering:

    • [lib] now appears immediately after [package] (before [features])
    • [patch.crates-io] moved after [workspace.lints.*] sections
    • Profile sections reordered: [profile.dev] before [profile.dev.package."*"]
    • [lints] moved to end of file
  2. Dependency alphabetization:

    • Root Cargo.toml:73-77: serde-wasm-bindgen now correctly sorted with other serde_* crates
    • crates/bitwarden-uniffi/Cargo.toml: Android dependencies alphabetically sorted
  3. Trailing comma additions:

    • Feature arrays now have trailing commas consistently (improves diffs for future changes)

🔍 Verification of Critical Changes

Workflow file (.github/workflows/lint.yml):

  • ✅ Line 90: Correctly changed from git installation to cargo install cargo-sort --version 2.0.2 --locked
  • ✅ Line 93: Correctly added --grouped parameter as required by new version

Root Cargo.toml:

  • ✅ Lines 73-77: serde-wasm-bindgen moved to alphabetical position (version unchanged)
  • ✅ Lines 102-125: Section reordering only, all content preserved

All crate Cargo.toml files:

  • ✅ Consistent pattern: [lib][features][dependencies][lints]
  • ✅ No dependency versions or feature flags modified

Summary

This is a clean PR that:

  • ✅ Updates tooling to an official release version (better than git-pinned)
  • ✅ Maintains backward compatibility (no API changes)
  • ✅ Improves developer experience by standardizing on a released version
  • ✅ Reduces drift when developers use newer cargo-sort versions locally
  • ✅ Documentation and CI properly updated for new requirements

No issues found. The PR is ready to merge.


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 upgrades the cargo-sort tool from a git revision to the official version 2.0.2 and adds the --grouped parameter to all cargo-sort invocations. The changes result in automated reformatting of Cargo.toml files throughout the repository with improved section ordering and consistent formatting.

Key changes:

  • Updated cargo-sort installation to use version 2.0.2 instead of a specific git revision
  • Added --grouped parameter to all cargo-sort commands for better section grouping
  • Reformatted all Cargo.toml files with consistent alphabetical ordering, trailing commas, and standardized section order

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.

Show a summary per file
File Description
.github/workflows/lint.yml Updated CI workflow to install cargo-sort v2.0.2 and use --grouped flag
lint-staged.config.js Added --grouped parameter to pre-commit cargo sort command
README.md Updated documentation to include --grouped flag in lint check example
Cargo.toml Reordered workspace dependencies alphabetically and reorganized profile sections
crates/bitwarden-wasm-internal/Cargo.toml Moved [features] section after [lib] section
crates/bitwarden-vault/Cargo.toml Added trailing commas to feature dependency lists
crates/bitwarden-uuid/Cargo.toml Moved [lints] section to end of file
crates/bitwarden-uuid-macro/Cargo.toml Moved [lib] section before [dependencies]
crates/bitwarden-uniffi/Cargo.toml Moved [features] after [lib] and reordered target-specific dependencies
crates/bitwarden-threading/Cargo.toml Added trailing comma to wasm feature list
crates/bitwarden-state/Cargo.toml Reordered target-specific dependency sections
crates/bitwarden-ssh/Cargo.toml Added trailing comma to wasm feature dependencies
crates/bitwarden-send/Cargo.toml Added trailing comma to uniffi feature list
crates/bitwarden-pm/Cargo.toml Added trailing commas to all feature dependency lists
crates/bitwarden-ipc/Cargo.toml Added trailing comma to wasm feature list
crates/bitwarden-generators/Cargo.toml Added trailing comma to wasm feature dependencies
crates/bitwarden-exporters/Cargo.toml Added trailing comma to wasm feature list
crates/bitwarden-error/Cargo.toml Moved [lints] section to end of file
crates/bitwarden-error-macro/Cargo.toml Moved [lib] before [features] and [lints] to end
crates/bitwarden-crypto/Cargo.toml Added trailing comma to uniffi feature dependencies
crates/bitwarden-core/Cargo.toml Added trailing commas to all feature dependency lists
crates/bitwarden-collections/Cargo.toml Added trailing comma to uniffi feature list
crates/bitwarden-auth/Cargo.toml Added trailing comma to wasm feature dependencies
bitwarden_license/bitwarden-commercial-vault/Cargo.toml Added trailing comma to wasm feature list

Copy link
Contributor

@quexten quexten left a comment

Choose a reason for hiding this comment

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

Approving for KM

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.

5 participants