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

Get reports with details by default #333

Merged
merged 9 commits into from
Nov 14, 2020

Conversation

y0urself
Copy link
Member

@y0urself y0urself commented Nov 12, 2020

What:

In get_report is details parameter is True on default now ...

Why:

It is frequently asked, why the report does not contain any information.

How:

Checklist:

@y0urself y0urself requested a review from a team as a code owner November 12, 2020 17:24
@codecov
Copy link

codecov bot commented Nov 12, 2020

Codecov Report

Merging #333 (89fabbe) into master (d635e58) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #333      +/-   ##
==========================================
- Coverage   97.47%   97.47%   -0.01%     
==========================================
  Files          20       20              
  Lines        4235     4234       -1     
==========================================
- Hits         4128     4127       -1     
  Misses        107      107              
Impacted Files Coverage Δ
gvm/protocols/gmpv7/gmpv7.py 99.15% <100.00%> (-0.01%) ⬇️

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 d635e58...89fabbe. Read the comment docs.

@bjoernricks
Copy link
Contributor

As your comment already suggests setting details to True by default should be the better solution. The comment should be updated to mention the new default then too.

Copy link
Contributor

@bjoernricks bjoernricks left a comment

Choose a reason for hiding this comment

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

details should be true instead of logging an info message. Setting sane defaults seems to be better to me.

Copy link
Contributor

@bjoernricks bjoernricks left a comment

Choose a reason for hiding this comment

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

Why not just changing the method declaration to details: Optional[bool] = True?

@bjoernricks bjoernricks changed the title Adding an info message if get_report gets called without details Get reports with details by default Nov 14, 2020
@bjoernricks bjoernricks merged commit fd2ed85 into greenbone:master Nov 14, 2020
@y0urself y0urself deleted the print-warning branch May 20, 2021 16:42
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