Skip to content
This repository has been archived by the owner on Apr 9, 2024. It is now read-only.

Fix #15, fix #16, fix #17 #18

Merged
merged 3 commits into from Dec 6, 2019
Merged

Fix #15, fix #16, fix #17 #18

merged 3 commits into from Dec 6, 2019

Conversation

ghost
Copy link

@ghost ghost commented Dec 6, 2019

#15: Suggestion: populate result.message.markdown.
#16: Suggestion: populate rule metadata with helpUri.
#17: Bug: uriBaseId is a symbolic name, not a path.

@michaelcfanning @pascalberger

@codecov
Copy link

codecov bot commented Dec 6, 2019

Codecov Report

Merging #18 into release/0.8.0 will increase coverage by 5.51%.
The diff coverage is 96.55%.

Impacted file tree graph

@@                Coverage Diff                @@
##           release/0.8.0      #18      +/-   ##
=================================================
+ Coverage          76.84%   82.35%   +5.51%     
=================================================
  Files                  3        3              
  Lines                 95      119      +24     
  Branches              10       13       +3     
=================================================
+ Hits                  73       98      +25     
+ Misses                13       12       -1     
  Partials               9        9
Impacted Files Coverage Δ
...rc/Cake.Issues.Reporting.Sarif/IIssueExtensions.cs 62.16% <100%> (ø) ⬆️
...ssues.Reporting.Sarif/SarifIssueReportGenerator.cs 97.4% <96.42%> (+3.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 307e211...f8b7fc9. Read the comment docs.

#15: Suggestion: populate result.message.markdown.
#16: Suggestion: populate rule metadata with helpUri.
#17: Bug: uriBaseId is a symbolic name, not a path.
@pascalberger pascalberger changed the base branch from develop to release/0.8.0 December 6, 2019 21:07
Copy link
Member

@pascalberger pascalberger left a comment

Choose a reason for hiding this comment

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

LGTM

@lgolding Thanks for your contribution. Very much appreciated. 👍

I already rebased you branch on the release/0.8.0 branch to prepare for merging, as the changes should go there. I saw that you just did another commit and merged the old changes in. I'll clean this up, when you think this PR is ready.

@ghost
Copy link
Author

ghost commented Dec 6, 2019

@pascalberger Thank you for the review! I also just pushed another tweak to the tests, verifying the correct values for result.level and result.kind.

@michaelcfanning and I would be very interested to know how you heard about SARIF, and what made you decide to adopt it. We're delighted to see spontaneous adoption!

@ghost
Copy link
Author

ghost commented Dec 6, 2019

@pascalberger Oh, and sorry, just to be clear -- yes, I do think the PR is ready now. You can merge whenever you like.

@pascalberger
Copy link
Member

@lgolding wrote:

@michaelcfanning and I would be very interested to know how you heard about SARIF, and what made you decide to adopt it. We're delighted to see spontaneous adoption!

Cake Issues is an extensible toolkit for issue management inside Cake build script, so having SARIF support seems like a natural fit. This addin provides export issues to SARIF format. I've planned to write another one which allows to import files in SARIF format, which would allow to e.g. run some linter which can create a SARIF compatible file in a CI build, read it and report the issues to the pull request (e.g. as comment in Azure DevOps).
Honestly no idea where I heard first about SARIF, as it was quite a while ago :)

@pascalberger pascalberger merged commit ce05e0a into cake-contrib:release/0.8.0 Dec 6, 2019
@pascalberger
Copy link
Member

@lgolding your changes have been merged, thanks for your contribution 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant