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

Add option to display line numbers that weren't executed #26

Merged
merged 1 commit into from
Jan 10, 2021

Conversation

flaeppe
Copy link
Contributor

@flaeppe flaeppe commented Oct 27, 2020

I took a swing at #16, displaying lines that weren't executed (uncovered lines) in the markdown table.

Added an option show_missing (default=false) for a column inclusion.

Although this doesn't include any fancy pants tracing of lines during branching
(e.g. misses on "jumps" that coveragepy outputs as: 22 -> 45)

Anyhow, let me know what you think. Great package. Thanks.

src/action.js Outdated Show resolved Hide resolved
@hannseman
Copy link
Contributor

It'll need a rebase and a npm run package to make the checks ✅

Copy link
Contributor

@hannseman hannseman left a comment

Choose a reason for hiding this comment

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

Just some nitpicks

src/cobertura.js Outdated Show resolved Hide resolved
src/cobertura.js Outdated Show resolved Hide resolved
src/cobertura.js Outdated Show resolved Hide resolved
src/cobertura.js Outdated Show resolved Hide resolved
@flaeppe
Copy link
Contributor Author

flaeppe commented Jan 10, 2021

Thanks for the review.

Seems that a coverage check is failing. Looks like it fails on parsing my new show_missing variable. Am I interpreting it correctly that a GitHub environment is missing the variable? It's set as required: true. If that is the case, maybe we should make it required: false for backwards compatibility? (Can't remember if GitHub actions is forcing you to explicitly set a version when using an external action package)

@hannseman
Copy link
Contributor

I get a YAML validation error on line 31 when I run it through a validator. Maybe some weird white space?

did not find expected key while parsing a block mapping

@github-actions
Copy link

github-actions bot commented Jan 10, 2021

File Coverage
All files 100%

Minimum allowed coverage is 75%

Generated by 🐒 cobertura-action against ddeab80

@flaeppe
Copy link
Contributor Author

flaeppe commented Jan 10, 2021

I get a YAML validation error on line 31 when I run it through a validator. Maybe some weird white space?

did not find expected key while parsing a block mapping

Read the file name in the error a bit too quickly, was looking in the wrong file. There was a misplaced escape character on the failing line.

Copy link
Contributor

@hannseman hannseman left a comment

Choose a reason for hiding this comment

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

Really awesome work! 💯

Just double-checking, did you run npm run package after the last changes? (Would be sweet if we could automate this.. At least it's better than keeping node_modules under version control)

@flaeppe
Copy link
Contributor Author

flaeppe commented Jan 10, 2021

Just double-checking, did you run npm run package after the last changes?

Yeah, should be all good. Ran it without any diff just now.

@hannseman hannseman merged commit 4c77ef9 into 5monkeys:master Jan 10, 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