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

feat(merge): Add filter for intermediate reports #1262

Merged
merged 4 commits into from
Aug 12, 2021

Conversation

imsahil007
Copy link
Contributor

I have added a filter for intermediate reports which will filter out reports from the merge output.
Do users want to filter out the reports on the basis of a date range as well? If that is the case, should I add -s --start-date and -e --end-date arguments. Or should I just convert -F --filter to a dictionary type input.?

@terriko
Copy link
Contributor

terriko commented Jul 27, 2021

Sorry, looks like merging #1218 made some minor merge conflicts here.

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

I'd like to have an entry in the manual explaining why the user would want this. Is the expectation that they'll have all the reports in a directory and merge them all every time, and might just want to exclude one without moving the file(s) with that tag out of the directory? What circumstances would make this useful? Are you thinking "what if you had nightly scans but you only wanted to show the ones per week" or something else?

@imsahil007 imsahil007 marked this pull request as draft July 28, 2021 15:53
@codecov-commenter
Copy link

codecov-commenter commented Aug 11, 2021

Codecov Report

Merging #1262 (8dd7e75) into main (82de345) will decrease coverage by 0.03%.
The diff coverage is 55.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1262      +/-   ##
==========================================
- Coverage   79.35%   79.31%   -0.04%     
==========================================
  Files         271      271              
  Lines        4902     4908       +6     
  Branches      591      593       +2     
==========================================
+ Hits         3890     3893       +3     
- Misses        865      866       +1     
- Partials      147      149       +2     
Flag Coverage Δ
longtests 79.31% <55.55%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cve_bin_tool/cli.py 74.26% <25.00%> (-0.74%) ⬇️
cve_bin_tool/merge.py 79.50% <80.00%> (-0.33%) ⬇️

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 82de345...8dd7e75. Read the comment docs.

@imsahil007 imsahil007 marked this pull request as ready for review August 11, 2021 15:46
@terriko
Copy link
Contributor

terriko commented Aug 12, 2021

Looks good! I really like the new docs. We might want to consider putting --merge and --filter into their own argument group to make it more clear that they go together, but I'll file a separate issue for that.

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.

3 participants