Skip to content
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

perf: Improve copy_file.bzl's progress_message and do some cleanup #931

Merged
merged 3 commits into from
Sep 10, 2024

Conversation

dzbarsky
Copy link
Contributor

@dzbarsky dzbarsky commented Sep 7, 2024

The change to progress_message saves around 100ms of analysis time for my big repo. The rest of the changes are cleanups to reflect that we have long since diverged from skylib's implementation.

@dzbarsky dzbarsky changed the title Cleanup copy_file.bzl perf: Improve copy_file.bzl's progress_message and do some cleanup Sep 7, 2024
@jbedard jbedard requested a review from alexeagle September 7, 2024 18:42
@jbedard
Copy link
Collaborator

jbedard commented Sep 7, 2024

This code removal on its own seems worth it to me, but even more so for the perf win (even if small).

@alexeagle is an extra ../ (sometimes?) showing up in a progress message a big deal?

@jbedard jbedard merged commit 4c1267f into bazel-contrib:main Sep 10, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants