-
Notifications
You must be signed in to change notification settings - Fork 104
feat: support stampinf version override #520
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
feat: support stampinf version override #520
Conversation
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 pull request introduces conditional support for overriding the DriverVer version in the stampinf tool through an environment variable when unstable build flags are enabled.
- Adds conditional logic to allow
STAMPINF_VERSIONenvironment variable to override auto-generated driver version - Implements parameterized testing to verify the new conditional behavior
- Updates configuration to support linting and documentation of the new unstable cfg flag
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/cargo-wdk/src/actions/build/package_task.rs | Implements conditional DriverVer override logic and adds comprehensive test coverage |
| crates/cargo-wdk/Cargo.toml | Adds linting configuration for unexpected cfgs including the new flag |
| .cargo/config.toml | Documents the new unstable cfg option for discoverability |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
remove redundant comment Co-authored-by: Copilot <[email protected]> Signed-off-by: Krishna Kumar Thokala <[email protected]>
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
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]> Signed-off-by: Krishna Kumar Thokala <[email protected]>
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
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
… run_stampinf test
…t variable handling
…ndling in run_stampinf
…ith_env utility function
… cleanup functionality
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
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
@krishnakumar4a4 looks okay to me as long as you address remaining comments from @wmmc88 and Copilot
…n assertion functions
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
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…in test utility functions
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
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
leon-xd
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
This pull request introduces improved handling of the
STAMPINF_VERSIONenvironment variable for the driver packaging process, ensuring that thestampinftool correctly sets theDriverVerfield when the environment variable is present. It also adds a robust utility for managing environment variables in tests, and updates existing tests to use this utility to ensure isolation and reliability. Additionally, a new test is added to verify theSTAMPINF_VERSIONoverride logic. The changes also add theregexcrate as a workspace dependency.Environment variable handling and testing improvements:
with_envutility function for safely setting and restoring environment variables in tests, ensuring test isolation and preventing cross-test contamination. This function is now used throughout all relevant tests. (crates/cargo-wdk/src/main.rs,crates/cargo-wdk/src/actions/build/tests.rs) [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18]STAMPINF_VERSION environment variable support:
run_stampinflogic so that if theSTAMPINF_VERSIONenvironment variable is set and non-empty, the-vargument is omitted, allowingstampinfto read and use the environment variable for theDriverVerfield. Otherwise, the default-v *argument is used. (crates/cargo-wdk/src/actions/build/package_task.rs) [1] [2]STAMPINF_VERSIONcorrectly influences the arguments passed tostampinf. (crates/cargo-wdk/src/actions/build/package_task.rs)Dependency management:
regexcrate as a workspace dependency inCargo.tomlfor future or current use. (crates/cargo-wdk/Cargo.toml)Integration test updates:
with_envutility and to defineSTAMPINF_VERSION_ENV_VARfor consistency. (crates/cargo-wdk/tests/build_command_test.rs)These changes collectively improve the reliability of the build process when environment variables are involved, and make the test suite more robust and maintainable.