-
Notifications
You must be signed in to change notification settings - Fork 110
fix: remove --cwd arg from cargo-wdk
#437
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
Conversation
f85beda to
ef59e19
Compare
There was a problem hiding this 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 removes the --cwd argument from cargo-wdk to better align with standard cargo build UX, which uses --manifest-path instead of --cwd for specifying project paths.
- Removes
--cwdCLI argument definition and related code - Updates tests to use
current_dir()method instead of--cwdflag - Updates documentation to remove
--cwdusage examples
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/cargo-wdk/src/cli.rs | Removes --cwd field from BuildArgs struct and hardcodes working directory to current path |
| crates/cargo-wdk/tests/build_command_test.rs | Updates test to use current_dir() instead of --cwd argument |
| crates/cargo-wdk/tests/new_command_test.rs | Updates test to use current_dir() instead of --cwd argument |
| crates/cargo-wdk/README.md | Removes documentation example showing --cwd usage |
63a5cc0 to
d4e8bd7
Compare
`--cwd` is not an argument on `cargo build`. It has `--manifest-path` instead. Since we are emulating `cargo build` we should expose the same UX. This PR only removes `--cwd`. Will add `--manifest-path` at some later point because it is not something very commonly used.
|
@gurry There is one left over mention of |
Done |
krishnakumar4a4
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* fix: remove `--cwd` arg from `cargo-wdk` (microsoft#437) * Initial plan * Switch GitHub Actions runner from windows-latest to windows-2025 - Update all runs-on declarations from windows-latest to windows-2025 across 8 workflow files - Update corresponding comments referencing windows-latest for consistency - Maintain x64 architecture assumption as windows-2025 is also x64-based This change provides access to Windows Server 2025 which includes winget pre-installed, improving the CI setup and build performance. Co-authored-by: wmmc88 <[email protected]> * Replace winget-install action with direct PowerShell module installation for windows-2025 Co-authored-by: wmmc88 <[email protected]> * improve install-wdk action to prevent header conflicts by cleaning existing WDK/SDK installations Co-authored-by: wmmc88 <[email protected]> * Fix WDK installation pipeline errors by importing PowerShell module and improving error handling Co-authored-by: wmmc88 <[email protected]> * Fix PowerShell syntax error in install-wdk action by properly escaping $_ variable references Co-authored-by: wmmc88 <[email protected]> --------- Co-authored-by: Gurinder Singh <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: wmmc88 <[email protected]>
--cwdis not an argument oncargo buildwhich uses--manifest-pathinstead. Since we are emulatingcargo buildwe should expose the same UX.This PR only removes
--cwd. Will add--manifest-pathat some later point because it is not something all that commonly used.