-
Notifications
You must be signed in to change notification settings - Fork 1
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
Improve performance of VCS diff listing #343
Improve performance of VCS diff listing #343
Conversation
Created using spr 1.3.5
|
Improves performance of listing yarn lock files and VCS changes by: * changing exclude patterns * using rayon Test Plan: cargo test Pull Request: #343
@@ -148,11 +149,15 @@ impl VCSState { | |||
exclude_patterns: &[String], | |||
failure_mode: FailureMode, | |||
) -> anyhow::Result<VCSState> { | |||
tracing::info!("Reading VCS state"); |
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.
for my understanding, why do we refer the "full path" rather than doing use tracing::{debug, info, error};
so that we can avoid repeating tracing::
?
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.
I haven't thought too much about it but I generally have used the full path because error
, info
, etc. are common names that may be shadowed.
let mut command = Command::new("git"); | ||
command | ||
.arg("ls-files") | ||
.arg("--cached") |
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.
Is it how we are going to report also staged files?
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.
No, this is just to list the yarn.lock files committed
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.
Are we dealing with staged files in a follow up PR?
The command in line 366 will report only unstaged
Improves performance of listing yarn lock files and VCS changes by:
Test Plan: cargo test