Skip to content

Don't pass ignored arguments to JaCoCo plugin #26340

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

Closed
wants to merge 3 commits into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Jun 19, 2025

JacocoInstrumentationProcessor ignores all but the first argument.

Work towards #5716

JacocoInstrumentationProcessor ignores all but the first argument.
@fmeum fmeum force-pushed the 5716-java-coverage-cleanup branch from edee190 to 3cb6a3a Compare June 19, 2025 07:28
@fmeum fmeum marked this pull request as ready for review June 19, 2025 08:07
@fmeum fmeum requested review from a team and lberki as code owners June 19, 2025 08:07
@fmeum fmeum requested review from gregestren and c-mita and removed request for a team, lberki and gregestren June 19, 2025 08:07
@github-actions github-actions bot added team-Configurability platforms, toolchains, cquery, select(), config transitions team-Rules-Java Issues for Java rules awaiting-review PR is awaiting review from an assigned reviewer labels Jun 19, 2025
@fmeum
Copy link
Collaborator Author

fmeum commented Jun 19, 2025

CI failures are unrelated and also happen on master.

@fmeum fmeum requested a review from lberki June 19, 2025 10:52
@fmeum
Copy link
Collaborator Author

fmeum commented Jun 19, 2025

FYI @lberki for dropping the coverage metadata root

@c-mita
Copy link
Member

c-mita commented Jun 24, 2025

@fmeum
Copy link
Collaborator Author

fmeum commented Jun 24, 2025

You could also delete the comment here https://cs.opensource.google/bazel/bazel/+/master:src/java_tools/buildjar/java/com/google/devtools/build/buildjar/instrumentation/JacocoInstrumentationProcessor.java;l=50 ?

I'm doing that in #26349, but I can split out that part and move it here. What do you prefer?

@c-mita
Copy link
Member

c-mita commented Jun 24, 2025

You could also delete the comment here https://cs.opensource.google/bazel/bazel/+/master:src/java_tools/buildjar/java/com/google/devtools/build/buildjar/instrumentation/JacocoInstrumentationProcessor.java;l=50 ?

I'm doing that in #26349, but I can split out that part and move it here. What do you prefer?

I think it would be better have it here, since it is related and it makes it clear what's going on for code review.

@fmeum fmeum requested a review from a team as a code owner June 24, 2025 14:11
@fmeum
Copy link
Collaborator Author

fmeum commented Jun 24, 2025

I think it would be better have it here, since it is related and it makes it clear what's going on for code review.

@c-mita Done

Copy link
Member

@c-mita c-mita left a comment

Choose a reason for hiding this comment

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

LGTM

@c-mita c-mita added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jun 24, 2025
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jun 26, 2025
@fmeum fmeum deleted the 5716-java-coverage-cleanup branch June 26, 2025 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Configurability platforms, toolchains, cquery, select(), config transitions team-Rules-Java Issues for Java rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants