-
Notifications
You must be signed in to change notification settings - Fork 204
GitHub annotations #432
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
GitHub annotations #432
Changes from 22 commits
6e573d4
a839043
a4ed797
7356129
7824b0b
d9cbac8
8b01b15
452f2d6
5c97184
c0a5ab1
d107e59
ab4bff4
13e64c8
70c9e08
7836cb0
d45ebb1
d7bf437
cc9c5b0
7a1e06f
38ac10f
97d77be
30bbc71
1271b58
b78f0f6
392dc17
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |||||
| from diff_cover.git_diff import GitDiffFileTool, GitDiffTool | ||||||
| from diff_cover.git_path import GitPathTool | ||||||
| from diff_cover.report_generator import ( | ||||||
| GitHubAnnotationsReportGenerator, | ||||||
| HtmlReportGenerator, | ||||||
| JsonReportGenerator, | ||||||
| MarkdownReportGenerator, | ||||||
|
|
@@ -27,6 +28,7 @@ | |||||
| HTML_REPORT_DEFAULT_PATH = "diff-cover.html" | ||||||
| JSON_REPORT_DEFAULT_PATH = "diff-cover.json" | ||||||
| MARKDOWN_REPORT_DEFAULT_PATH = "diff-cover.md" | ||||||
| GITHUB_ANNOTATIONS_DEFAULT_TYPE = "warning" | ||||||
| COMPARE_BRANCH_HELP = "Branch to compare" | ||||||
| CSS_FILE_HELP = "Write CSS into an external file" | ||||||
| FAIL_UNDER_HELP = ( | ||||||
|
|
@@ -266,9 +268,18 @@ def generate_coverage_report( | |||||
| if "markdown" in report_formats: | ||||||
| markdown_report = report_formats["markdown"] or MARKDOWN_REPORT_DEFAULT_PATH | ||||||
| reporter = MarkdownReportGenerator(coverage, diff) | ||||||
| with open(markdown_report, "wb") as output_file: | ||||||
| with open_file(markdown_report, "wb") as output_file: | ||||||
|
||||||
| with open_file(markdown_report, "wb") as output_file: | |
| with open(markdown_report, "wb") as output_file: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah - I merged in your branch - it was your change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed here: #516
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| {% if src_stats %} | ||
| {% for src_path, stats in src_stats|dictsort %} | ||
| {% if stats.percent_covered < 100 %} | ||
| {% for line in stats.violation_lines %} | ||
| {% set splitLines = line.split("-") %} | ||
| ::{{ annotations_type }} file={{ src_path }},line={{ splitLines[0] }}{% if splitLines[1] %},endLine={{ splitLines[1] }}{% endif %},title=Missing Coverage::Line {{ line }} missing coverage | ||
| {% endfor %} | ||
| {% endif %} | ||
| {% endfor %} | ||
| {% endif %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with adding
error. But i didn't want to impose itThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Bachmann1234 what do you think this should be?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will change it to use the default, which will be warning.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no "default" it will crash. This is a bigger bug that has little to do with your PR. For now, why dont we use
warningand wait for @Bachmann1234There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah - I see. I thought it would be a good test of the default, and in a way it was haha.