-
Notifications
You must be signed in to change notification settings - Fork 38
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: match before checking cache #447
Conversation
I just realized I can further simplify the stats since |
0e8ffff
to
6bc953f
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.
This looks great, thanks for making the change =)
I just realized I can further simplify the stats since matched and formatted will always be the same.
Sorry, I think I'm not understanding the vocabulary. Here's what I thought:
- matched: the number of files in the repo that treefmt "has opinions" about
- formatted: of the files we matched, the number of files that treefmt invoked a formatter on. caching means we may choose not to format a matched file (because we know it hasn't changed).
- changed: of the files we formatted, how many of them actually materially changed
This is a good summary. |
Sorry, I didn't get to my point, haha:
Is this true? When we have cache hits, won't we have files that we match that we end up not formatting? |
You're not wrong. Now that I look at this again, I have no idea why I removed the distinction. I think I need to take a break from treefmt for a few days after this. Putting it back in. |
7b88088
to
080888c
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.
LGTM!
080888c
to
52ac241
Compare
This changes the flow of processing to make unmatched behaviour more consistent. Before, we had been: - traversing the filesystem - comparing with the cache and only emitting files which had changed - applying the matching rules to determine which formatters should be applied to a given file - applying the formatters Now, we do the following: - traverse the filesystem - apply the matching rules to determine which formatters should be applied to a given file - compare with the cache and only emit files which have changed for formatting - apply the formatters It does mean we are applying the matching rules against files which we may not have to format, but in testing against Nixpkgs the performance impact appears negligible. This makes sense since most of the processing time will be spent in the formatters, not applying some globs to file paths. Signed-off-by: Brian McGee <[email protected]>
52ac241
to
71b262f
Compare
This changes the flow of processing to make unmatched behaviour more consistent.
Before, we had been:
Now, we do the following:
It does mean we are applying the matching rules against files which we may not have to format, but in testing against Nixpkgs the performance impact appears negligible.
This makes sense since most of the processing time will be spent in the formatters, not applying some globs to file paths.
Close #433