-
Notifications
You must be signed in to change notification settings - Fork 32
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
Implement Mandatory and Optional fields functionality #1665
Implement Mandatory and Optional fields functionality #1665
Conversation
7846b3f
to
fc20b17
Compare
@@ -6,8 +6,6 @@ Include paths: | |||
- wazuh_db: | |||
path: "../../tests/integration/test_wazuh_db" | |||
recursive: false | |||
- vulnerability_detector: |
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 think we should use the same separator. Underscore instead of space in all keys.
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.
Did you refer to the keys for other items in the config?
I think using spaces is more readable as it isn't required to be a unique word.
In this case, wazuh_db or vulnerability_detector refers to a Wazuh module.
- 0 | ||
- 1 | ||
Tags: | ||
- Enrollment |
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.
Since the tags are optional, I assume we are not going to validate them. Should be a problem to accept any word (or phrase?) as tags? I think about capital letters, misspelling words like enrrolment
instead of Enrollment
, etc.
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 agree. But, the requirement is to accept any tag found in the documentation. During the sanity check stage, all the tags found will be listed as a way to identify this kind of problem.
On the other hand, we can propose to move every tag to a specific case.
docs/DocGenerator/config.yaml
Outdated
- One | ||
- Two | ||
- Metadata | ||
- Modules |
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.
Isn't it a little bit confusing to have the metadata
fields out of it?
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.
This change was aimed to simplify the configuration file and make it more flexible. Having a tree of allowed values was more strict to write in the documentation and harder to parse in the code.
We will check if it is required to modify this.
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.
Now, the Optional and Mandatory fields have a tree format as suggested.
51af91a
to
eca8835
Compare
from docstring_parser import parse | ||
from comment_parser import comment_parser | ||
import warnings | ||
|
||
INTERNAL_FIELDS = ['Id', 'Group Id', 'Name'] |
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.
Since we agreed to use snake case for those fields, we should change this.
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.
These changes were found and requested during the exploratory testing.
So they are included in this PR
from docstring_parser import parse | ||
from comment_parser import comment_parser | ||
import warnings | ||
|
||
INTERNAL_FIELDS = ['Id', 'Group Id', 'Name'] | ||
STOP_FIELDS = ['Tests','Test Cases'] |
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.
... And this
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.
Ditto
Description
This PR adds a configurable way to decide which fields must be included in the documentation and which must be checked during a sanity scan.
Each field not found in the Mandatory or the Optional fields will be removed from the documentation output.
Each field in the Mandatory fields will be checked during the Sanity execution.
This is implemented for modules and tests separately.
A new Utils module was included with useful dictionary handling functions.
Also, some example documentation blocks were added to WazuhDB test.