Skip to content
This repository was archived by the owner on Nov 29, 2021. It is now read-only.

Start with param - #31

Merged
janowagner merged 5 commits into
greenbone:masterfrom
jjnicola:start-with-param
Jul 12, 2018
Merged

Start with param#31
janowagner merged 5 commits into
greenbone:masterfrom
jjnicola:start-with-param

Conversation

@jjnicola

Copy link
Copy Markdown
Member

Add vts params to start_scan as a vt subelement.
Now each vt is a subelement of . It has an id attribute. Each vt accept multiple optional vt_param as subelements, with name and type as attribute and the value as text.
Update example for start_scan with vts parameters.
Add unittest to test VTs as elements and VT's parameters.

jjnicola added 3 commits July 10, 2018 15:30
Now each vt is a subelement of <vts>. It has an id attribute.
Each vt accept multiple optional vt_param as subelements, with
name and type as attribute and the value as text.
Update example for start_scan with vts parameters
@jjnicola
jjnicola requested review from a team and janowagner July 11, 2018 10:36

@janowagner janowagner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see three things to improve:

  1. The use of variable "id", see the CI error.
  2. Don't use ET.fromstring even in the tests module,
    like I also mentioned in the other PR where secET
    was introduced (partially).
  3. "process something" is neither a helpful
    command name and even more it is not helpful
    as a function documentation. Please explain in
    more detail in the doc string what "process it"
    actually means.

@jjnicola jjnicola mentioned this pull request Jul 12, 2018
@jjnicola

Copy link
Copy Markdown
Member Author

I have done the improvements 1) and 3) . Regarding the point 2), as I explained in PR #30, I will do a new PR with the requested changes for the tests, to avoid merge conflicts.

@jjnicola
jjnicola requested a review from janowagner July 12, 2018 11:40

@janowagner janowagner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the 2 solved items.
So, lets approve this PR and as a follow-up then
use secure XML parsing in the tests.

@janowagner
janowagner merged commit a8c94d4 into greenbone:master Jul 12, 2018
@jjnicola
jjnicola deleted the start-with-param branch July 13, 2018 08:20
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.

2 participants