Skip to content

coverage action needs cleanup #1818

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
drammock opened this issue May 16, 2024 · 1 comment
Closed

coverage action needs cleanup #1818

drammock opened this issue May 16, 2024 · 1 comment
Labels
impact: block-release Should block a release from happening. Only use if this is a critical problem we don't want to ship kind: maintenance Improving maintainability and reducing technical debt tag: CI Pull requests that update GitHub Actions code

Comments

@drammock
Copy link
Collaborator

The problems I see so far:

  1. including coverage comment step in tests.yml breaks the publish workflow (as it's triggered on releases, not PRs, so there's no PR for it to write a comment to).

  2. There's a dedicated workflow file for the coverage comment and also a step inside tests.yml that appears to do the same thing (though their configs are not identical):

- name: Coverage comment
id: coverage_comment
uses: py-cov-action/python-coverage-comment-action@v3
with:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
MERGE_COVERAGE_FILES: true

These should be deduplicated.

  1. IMO, ideally the release workflow wouldn't run coverage-related steps at all (only tests).
@drammock drammock added kind: maintenance Improving maintainability and reducing technical debt tag: CI Pull requests that update GitHub Actions code labels May 16, 2024
@drammock drammock mentioned this issue May 16, 2024
10 tasks
@drammock drammock added the impact: block-release Should block a release from happening. Only use if this is a critical problem we don't want to ship label May 16, 2024
Carreau pushed a commit that referenced this issue May 27, 2024
As I was checking #1818 and the new workflows, I realized I had forgotten to update the workflow reference so that the coverage action calls the correct workflow.

AFAIK the way the workflows are implemented now, the coverage comment should only trigger on PRs, so should no longer block releases, as mentioned in #1818
@trallard
Copy link
Collaborator

AFAIK this is already addressed by the last two CI PRs. So will close the issue but we can reopen if it turns this still needs work

ivanov pushed a commit to ivanov/pydata-sphinx-theme that referenced this issue Jun 5, 2024
As I was checking pydata#1818 and the new workflows, I realized I had forgotten to update the workflow reference so that the coverage action calls the correct workflow.

AFAIK the way the workflows are implemented now, the coverage comment should only trigger on PRs, so should no longer block releases, as mentioned in pydata#1818
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact: block-release Should block a release from happening. Only use if this is a critical problem we don't want to ship kind: maintenance Improving maintainability and reducing technical debt tag: CI Pull requests that update GitHub Actions code
Projects
None yet
Development

No branches or pull requests

2 participants