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

Raise error if declared coverage not found #135

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lazyatom
Copy link
Contributor

We can get the "build" to warn us and show the exception(s) if the expected coverage data is missing (or has a typo, more likely).

We can get the "build" to warn us and show the exception(s) if the
expected coverage data is missing (or has a typo, more likely).
@lazyatom lazyatom requested a review from h-lame October 14, 2020 09:53
@h-lame
Copy link
Member

h-lame commented Oct 14, 2020

Interesting - I have thought about just putting in all the {::coverage ...} tags so that folk raising PRs to add coverage just need to update the data and this'd break that possibility, but I can certainly see the appeal of this - we don't have much in the way of checks that this site actually does what we expect.

@h-lame
Copy link
Member

h-lame commented Oct 15, 2020

Actually, I'm reminded of what a useful failure case might be; rather than complaining if coverage can't be found, what if we linted the json / yml files in data to make sure they're valid. Last month I asked a speaker to add a coverage link and their json had an extra comma in it, which I only spotted post-merge.

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