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

Only emit aspect_rules_js_metadata when it's required #1420

Merged
merged 1 commit into from
Jan 6, 2024

Conversation

dzbarsky
Copy link
Collaborator

Type of change

- Performance (a code change that improves performance)

Test plan

- Covered by existing test cases

My understanding is this file is only consumed by actions with a non-empty file, so writing an empty one is unnecessary. This saves 2.5% of files in the npm runfiles tree on typical test actions which translates to a small improvement in test runtime.
As another datapoint, in bazelbuild/bazel#8230 (comment) I computed that the average NPM package we use has around 45 files. 44/45 = 97.8% :)

@jbedard jbedard requested a review from gregmagolan January 2, 2024 06:27
Copy link
Member

@gregmagolan gregmagolan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@dzbarsky dzbarsky force-pushed the zbarsky/aspect_metadat branch from 815c6dc to b8e0961 Compare January 5, 2024 15:06
@jbedard jbedard merged commit 42b751a into aspect-build:main Jan 6, 2024
95 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants