Skip to content

feat(cargo-wdk): support hyphen-prefixed signtool args - #737

Open
Shravan Vasista (svasista-ms) wants to merge 2 commits into
microsoft:mainfrom
svasista-ms:feat/signtool-args
Open

feat(cargo-wdk): support hyphen-prefixed signtool args#737
Shravan Vasista (svasista-ms) wants to merge 2 commits into
microsoft:mainfrom
svasista-ms:feat/signtool-args

Conversation

@svasista-ms

Copy link
Copy Markdown
Contributor

This PR adds support for hyphen prefixed --signtool-args values. For e.g. -s WDRCustomTestStore /n WDRCustomTestCert -fd SHA256.

Signtool supports both - and / prefixed arguments. But cargo-wdk currently does not support - prefixed args for signtool-args. In this PR, this support is added via the allow_hyphen_values attribute on signtool_args field. It also makes the field Option<Vec<String>> similar to other passthrough args.

Copilot AI lite review requested due to automatic review settings September 11, 2026 06:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Hyphen-prefixed password arguments are not currently redacted from logs and error messages.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adds support for hyphen-prefixed signtool arguments and optional passthrough signing arguments.

Changes:

  • Enables --prefixed --signtool-args values.
  • Changes signing arguments to Option<Vec<String>>.
  • Updates unit and integration test coverage.
File summaries
File Description
crates/cargo-wdk/tests/build_command_test.rs Tests hyphen-prefixed signing arguments.
crates/cargo-wdk/src/cli.rs Parses hyphen-prefixed signing arguments.
crates/cargo-wdk/src/actions/build/tests.rs Updates signing test fixtures.
crates/cargo-wdk/src/actions/build/package_task.rs Handles optional signing arguments.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread crates/cargo-wdk/src/cli.rs
@codecov-commenter

Codecov Comments Bot (codecov-commenter) commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.14286% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.51%. Comparing base (343c81c) to head (b91932b).

Files with missing lines Patch % Lines
crates/cargo-wdk/src/actions/build/package_task.rs 96.87% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #737      +/-   ##
==========================================
- Coverage   78.52%   78.51%   -0.01%     
==========================================
  Files          30       30              
  Lines        7105     7103       -2     
  Branches     7105     7103       -2     
==========================================
- Hits         5579     5577       -2     
  Misses       1407     1407              
  Partials      119      119              

☔ View full report in Codecov by Harness.
📢 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.

Copilot AI review requested due to automatic review settings September 11, 2026 06:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No unresolved review comments remain, and the supplied assessments indicate the changes are ready for approval.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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