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.

Sign up for GitHub

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

Jump to bottom

Add coverage checks to CI #149

Merged
merged 1 commit into from
Jun 21, 2023
Merged

Add coverage checks to CI #149

merged 1 commit into from
Jun 21, 2023
+78 −2

Conversation

brandonwillard
Copy link
Member

Closes #123

@brandonwillard brandonwillard marked this pull request as draft June 19, 2023 17:43
@brandonwillard brandonwillard self-assigned this Jun 19, 2023
@brandonwillard brandonwillard added enhancement tests Linked to library tests labels Jun 19, 2023
@brandonwillard
Copy link
Member Author

@rlouf, looks like we're seeing an issue with HF code: here.

@brandonwillard brandonwillard force-pushed the add-ci-coverage branch 3 times, most recently from 4028108 to 9eb1a4b Compare June 20, 2023 16:31
@brandonwillard
Copy link
Member Author

@rlouf, OK, this seems to work: https://github.com/normal-computing/outlines/actions/runs/5325109338#summary-14415152531

Looks like we might want to go with our old diff-cover approach if we want relative coverage requirements, though.

@brandonwillard brandonwillard marked this pull request as ready for review June 20, 2023 19:03
rlouf
rlouf approved these changes Jun 21, 2023
View reviewed changes
Copy link
Member

@rlouf rlouf left a comment

Choose a reason for hiding this comment

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

Looks good to me. The lack of diff is annoying, but not as annoying as CodeCov's random failures. We can always add that later.

@brandonwillard
Copy link
Member Author

Just changed it to use diff-cover and produce Markdown reports (see the summary). I'll push a temporary change in a second and make sure it looks good.

@brandonwillard brandonwillard force-pushed the add-ci-coverage branch 2 times, most recently from adf8306 to f4061f9 Compare June 21, 2023 18:06
@brandonwillard
Copy link
Member Author

@rlouf, how does this summary look?

@rlouf
Copy link
Member

rlouf commented Jun 21, 2023

Looks pretty good!

brandonwillard
brandonwillard commented Jun 21, 2023
View reviewed changes
Copy link
Member Author

@brandonwillard brandonwillard left a comment

Choose a reason for hiding this comment

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

@rlouf, I've removed the test commit, so we can merge this now.

@rlouf rlouf merged commit 4e1016d into dottxt-ai:main Jun 21, 2023
@brandonwillard brandonwillard deleted the add-ci-coverage branch June 21, 2023 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rlouf rlouf rlouf approved these changes

Assignees

@brandonwillard brandonwillard

Labels
enhancement tests Linked to library tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add coverage to CI
2 participants
@brandonwillard @rlouf