Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[skip changelog] Enable Codecov comments on PRs from forks #1819

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

Merged
per1234 merged 1 commit into arduino:master from per1234:codecov-on-fork-prs
Aug 1, 2022
Merged

[skip changelog] Enable Codecov comments on PRs from forks #1819

per1234 merged 1 commit into arduino:master from per1234:codecov-on-fork-prs
Aug 1, 2022

Conversation

Copy link
Contributor

@per1234 per1234 commented Jul 31, 2022
edited
Loading

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • [N/A] Tests for the changes have been added (for bug fixes / features)
  • [N/A] Docs have been added / updated (for bug fixes / features)
  • [N/A] UPGRADING.md has been updated with a migration guide (for breaking changes)

What kind of change does this PR introduce?

Infrastructure enhancement

What is the current behavior?

Versions of the codecov/codecov-action GitHub Actions action prior to 1.0.6 required the use of a token provided by Codecov in order to upload coverage data to Codecov. This token was stored in a secret in the Arduino CLI repository and used in the test workflow.

For security reasons, secrets are not accessible when a workflow is triggered by an event generated by a fork of the repository. This meant that it was impossible to upload coverage data for the test runs triggered by PRs from forks (codecov/codecov-action#29). A conditional was added to the upload step of the workflow to cause it to only run on push event triggers, which effectively prevented its failure for runs on PRs from forks (#388).

The token requirement was removed in the 1.0.6 release of codecov/codecov-action, but the now pointless conditional was never removed from the workflow. This prevented PRs from forks from receiving the automated code coverage report comments that would otherwise encourage those contributors to resolve coverage deficiencies and facilitate the review process.

What is the new behavior?

The harmful conditional is removed from the coverage data upload steps of the workflow and PRs from forks will now receive coverage report comments, just as PRs from branches do already.

Does this PR introduce a breaking change

No breaking change.

Other information

The configuration of the coverage data upload step proposed here is aligned with our standardized "template" workflow:

https://github.com/arduino/tooling-project-assets/blob/d9f73eb4e5b16c0141e184d7f04493cd62221ea4/workflow-templates/test-go-task.yml#L104

That form of the workflow is already in use in several other Arduino tooling projects. For example:

Versions of the `codecov/codecov-action` GitHub Actions action prior to 1.0.6 required the use of a token provided by
Codecov in order to upload data to Codecov. This token was stored in secret in the Arduino CLI repository and used in
the test workflow.
For security reasons, secrets are not accessible when a workflow is triggered by an event generated by a fork of the
repository. This meant that it was impossible to upload coverage data for the test runs triggered by PRs from forks. A
conditional was added to the upload step of the workflow to cause it to only run on `push` event triggers, which
effectively prevented its failure for runs on PRs from forks.
The token requirement was removed in the 1.0.6 release of `codecov/codecov-action`, but the now pointless conditional
was never removed from the workflow. This prevented PRs from forks from receiving the automated code coverage report
comments that would otherwise encourage those contributors to resolve coverage deficiencies and facilitate the review
process.
The harmful conditional is hereby removed from the coverage data upload steps of the workflow and PRs from forks will
now receive coverage report comments, just as PRs from branches do already.
@per1234 per1234 added type: enhancement Proposed improvement topic: infrastructure Related to project infrastructure labels Jul 31, 2022
@per1234 per1234 self-assigned this Jul 31, 2022
Copy link

codecov bot commented Jul 31, 2022

Codecov Report

Merging #1819 (6fa9645) into master (5332ffd) will not change coverage.
The diff coverage is n/a.

@@ Coverage Diff @@
## master #1819 +/- ##
=======================================
 Coverage 35.53% 35.53% 
=======================================
 Files 231 231 
 Lines 19543 19543 
=======================================
 Hits 6945 6945 
 Misses 11773 11773 
 Partials 825 825 
Flag Coverage Δ
unit 35.53% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5332ffd...6fa9645. Read the comment docs.

per1234 and umbynos reacted with hooray emoji

Copy link
Contributor

@MatteoPologruto MatteoPologruto left a comment

Choose a reason for hiding this comment

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

Good job for spotting, highlighting and solving the problem! The PR's message is very clear.

per1234 and umbynos reacted with thumbs up emoji
@per1234 per1234 merged commit a55df0d into arduino:master Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MatteoPologruto MatteoPologruto MatteoPologruto approved these changes

@umbynos umbynos Awaiting requested review from umbynos

Labels
topic: infrastructure Related to project infrastructure type: enhancement Proposed improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /