-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Expose CoverageOutputGenerator on a Fragment #14724
Conversation
@c-mita can you review? |
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, maybe add a comment on the output_generator doc indicating it may be None, since I don't think that's obvious behaviour.
allowReturnNones = true, | ||
structField = true, | ||
doc = | ||
"Returns label pointed to by " |
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.
Can you add a note saying that this will be None if coverage collection isn't enabled?
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.
Done, I also slightly reworded the comment.
Ideally, I'd prefer this was guarded by an experimental flag, but I don't think that's feasible without making the feature clunky to use, so I'll let that pass. |
This allows Starlark rules to define _lcov_merger as a late-bound Starlark configuration_field in order not to pay the cost of building it when coverage isn't enabled.
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.
Thanks.
This allows Starlark rules to define _lcov_merger as a late-bound Starlark configuration_field in order not to pay the cost of building it when coverage isn't enabled. Fixes bazelbuild#8736 Fixes bazelbuild#10642 Closes bazelbuild#14724. RELNOTES: Add coverage configuration fragment, used to expose output_generator label. PiperOrigin-RevId: 433156089
This allows Starlark rules to define _lcov_merger as a late-bound Starlark configuration_field in order not to pay the cost of building it when coverage isn't enabled. Fixes #8736 Fixes #10642 Closes #14724. RELNOTES: Add coverage configuration fragment, used to expose output_generator label. PiperOrigin-RevId: 433156089
This allows Starlark rules to define _lcov_merger as a late-bound Starlark configuration_field in order not to pay the cost of building it when coverage isn't enabled.
Fixes #8736
Fixes #10642