refactor: improve logging for build action#495
Conversation
515c0e4 to
92d640b
Compare
There was a problem hiding this comment.
Pull Request Overview
This pull request improves logging consistency and conciseness throughout the build workflow by updating log messages across the cargo-wdk crate. The changes focus on making log output more streamlined and user-friendly while maintaining clarity about the build process status.
- Updated log messages to use more concise and consistent phrasing
- Simplified warning and info messages for better readability
- Updated test assertions to match the new log message format
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| build_command_test.rs | Updates test assertions to match new log message format from the build workflow |
| mod.rs | Streamlines log messages in build orchestration, making them more concise and consistent |
| build_task.rs | Simplifies cargo build log message to be more general and less verbose |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
gurry
left a comment
There was a problem hiding this comment.
We should also shorten this log line:
Processing Rust(possibly driver) project:
to this:
Processing project:
because the Rust(possible driver) part is a bit awkwardly worded and secondly not providing any useful information.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 4 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.
Signed-off-by: Gurinder Singh <gurisingh@microsoft.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Gurinder Singh <frederick.the.fool@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 4 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.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Gurinder Singh <frederick.the.fool@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new 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.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new 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.
Pull Request Overview
Copilot reviewed 5 out of 5 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.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new 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.
Pull Request Overview
Copilot reviewed 5 out of 5 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.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
wmmc88
left a comment
There was a problem hiding this comment.
good improvements. agree with some of the other comments that it could be better but i think it a good incremental improvement. Also it seems like the screenshots in the pr are not up to date with the latest code here (which is also why i suggested maybe snapshotting these types of tests is a better way forward medium term)
Yeah, I noticed the issues in the screenshots, especially the last one, and then fixed them in code, but was loath to update the screenshots again :D |
krishnakumar4a4
left a comment
There was a problem hiding this comment.
These log improvements look good to me. Thanks @gurry for taking this forward.
This pull request updates and streamlines log messages throughout the build workflow in the
cargo-wdkcrate. The changes focus on making the output more concise and consistent, removing redundant or overly verbose details, and improving clarity in log statements.Logging improvements:
Running cargo buildlog line that appeared before we invoke cargoRunning InfVerifif we are skipping it. We print onlyInvVerif skipped...in that case now.InvVerif skipped...log line from info to warn because it refers to an event that is unusualprocessingand replaced them with the more natural termbuildingThere is still more work to do in this area. The logging for instance can be confusing when building an emulated workspace -- it may be hard know which project is which. We will make such improvements later when we refactor cargo-wdk code which is also overdue.
Here are comparisons showing the effect of this PR on various build scenarios: standalone driver, workspace and emulated workspace. The left part is the output before this PR and the right side is after the PR.
Standalone Driver Project
Workspace
Emulated Workspace