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

Trim coverage report #8

Merged
merged 1 commit into from
May 4, 2024
Merged

Trim coverage report #8

merged 1 commit into from
May 4, 2024

Conversation

JustinKuli
Copy link
Owner

It's not clear why, but in the action run, the coverage report included a lot of dependencies. This grep might work.

It's not clear why, but in the action run, the coverage report included
a lot of dependencies. This grep might work.

Signed-off-by: Justin Kulikauskas <[email protected]>
Copy link

github-actions bot commented May 4, 2024

PR Review

⏱️ Estimated effort to review [1-5]

2, because the changes are limited to a specific workflow file and involve straightforward additions to filter and handle coverage reports. The logic is simple and the impact is confined to CI/CD pipeline behavior.

🏅 Score

85

🧪 Relevant tests

No

🔍 Possible issues

Possible Issue: The grep command is hardcoded to filter 'governance-policy-nucleus', which might not be flexible or applicable if the project structure or naming conventions change in the future.

🔒 Security concerns

No


✨ Review tool usage guide:

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

  • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

Comment on lines +41 to +44
- run: |
head -1 cover.out > nucleus_cover.out
grep 'governance-policy-nucleus' cover.out >> nucleus_cover.out
go tool cover -func=nucleus_cover.out
Copy link

Choose a reason for hiding this comment

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

Suggestion: It's recommended to use a more robust approach when filtering coverage data to avoid potential issues with filenames that could unintentionally match the grep pattern. Using a more specific pattern or a different method to identify the relevant sections of the coverage report could improve the reliability of the script. [enhancement]

Suggested change
- run: |
head -1 cover.out > nucleus_cover.out
grep 'governance-policy-nucleus' cover.out >> nucleus_cover.out
go tool cover -func=nucleus_cover.out
- run: |
head -1 cover.out > nucleus_cover.out
grep '^governance-policy-nucleus ' cover.out >> nucleus_cover.out
go tool cover -func=nucleus_cover.out

@JustinKuli JustinKuli merged commit 0ab3c4e into main May 4, 2024
9 checks passed
@JustinKuli JustinKuli deleted the fix-coverage-report branch May 4, 2024 20:15
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.

1 participant