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

Coverage ignore option #624

Closed
jtdLab opened this issue Sep 24, 2022 · 5 comments
Closed

Coverage ignore option #624

jtdLab opened this issue Sep 24, 2022 · 5 comments

Comments

@jtdLab
Copy link
Contributor

jtdLab commented Sep 24, 2022

Hi,

is it possible to add an option to combining_builder which leads to the line //coverage:ignore-file beeing added on top of generated files. Or is there already an existing solution?

Something like:

targets:
  $default:
    builders:
      source_gen:combining_builder:
        options:
          coverage-ignore-file: true
@jakemac53
Copy link
Contributor

jakemac53 commented Sep 26, 2022

It could be added (would work similar to ignore_for_file).

Note though that this would take effect for all source_gen builder outputs that are combined into that file, which might be fine, but is worth keeping in mind.

I would probably try and lean towards something a bit more general though than a coverage-specific thing. Not sure why ingore_for_file was implemented that way as a general approach would also have solved this use case :).

@jtdLab
Copy link
Contributor Author

jtdLab commented Sep 26, 2022

Yep i agree a general approach would be the preferable solution.

@jakemac53
Copy link
Contributor

I spoke a bit with @kevmoo and it sounds like we agree this is a sensible feature to have - but I don't think either of us have the time to implement it at this time. We would accept a PR though, either with a specific option to ignore coverage (maybe ignore_coverage: true?) or a general preamble option which accepts any text and inserts it at the top of the file. If we did the more general approach we would want to ensure the text ends with a newline.

jakemac53 pushed a commit that referenced this issue Oct 11, 2022
Adds a preamble to CombiningBuilder as discussed in (#624).
@kevmoo
Copy link
Member

kevmoo commented Jan 17, 2023

See #636 (comment)

@kevmoo kevmoo closed this as completed Jan 17, 2023
@kevmoo
Copy link
Member

kevmoo commented Jan 17, 2023

er, preamble

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

No branches or pull requests

3 participants