Skip to content

[skip changelog] Remove unnecessary token input from Codecov upload steps of test workflow #886

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
merged 2 commits into from
Aug 3, 2020
Merged

Conversation

per1234
Copy link
Contributor

@per1234 per1234 commented Aug 2, 2020

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
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • What kind of change does this PR introduce?

Feature.

  • What is the current behavior?

An outdated version of the codecov/codecov-action GitHub Actions action is pinned in the test workflow. This outdated version imposes a requirement for an upload token. This token is defined by a repository secret, which causes two issues:

  • The test workflow fails when run in forks of the repository because they are missing this secret. This spurious failure will cause confusion to contributors who are checking to be sure their contribution will pass CI.
  • We are prevented from configuring the workflow to trigger on pull_request events because GitHub Actions disables secrets when a workflow is triggered by an event from a fork.

Modern versions of the codecov/codecov-action action do not require the token when used in public repositories.

  • What is the new behavior?

The v1 ref of the codecov/codecov-action GitHub Actions action is specified by the test workflow. In addition to making the token unnecessary, this will also cause the workflow to automatically use the latest 1.x.x version of the action, so we will continue to benefit from the ongoing development work up to the point where any breaking changes cause them to do a major version bump.

The codecov/codecov-action action's token input is not used in the test workflow, eliminating the workflow's dependence on repository secrets.

  • Does this PR introduce a breaking change?

No.

per1234 added 2 commits August 2, 2020 09:38
Previously, the workflow pinned the outdated v1.0.2 ref. The requirement for an upload token has been removed since the 1.0.2 release. Removing the token input will provide several benefits:

- Prevent failures when the workflow is run in forks, which won't have the CODECOV_TOKEN secret defined.
- Allow configuring the workflow to be triggered by pull_request events, making it easy to evaluate the impact the pull request has on code coverage.

The use of the v1 ref, rather than pinning a specific version allows the workflow to automatically benefit from ongoing development work on the action, while still preventing it from being affected by breaking changes.
…flow

With the modern versions of the codecov/codecov-action GitHub Actions action, the upload token is only required for private repositories. Now that we are using a modern version of the action, this input only has a harmful effect by causing the workflow to fail when run in forks and also preventing us from using Codecov to evaluate the impact on code coverage of pull requests.
Copy link
Contributor

@rsora rsora left a comment

Choose a reason for hiding this comment

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

@per1234 Please monitor in the next days the codecov reports to verify the effectiveness of this fix!
Thanks!

@per1234 per1234 merged commit fead7fd into arduino:master Aug 3, 2020
@per1234 per1234 deleted the remove-codecov-token branch August 3, 2020 19:45
@per1234 per1234 self-assigned this Nov 23, 2021
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

Successfully merging this pull request may close these issues.

2 participants