OCPERT-56 OAR Fix wrong data types in docstring #462
Conversation
openshift-merge-robot
added
the
needs-rebase
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Hello @ltroiano-rh ,
There is redundant "." in your commit message.
Please rebase your pull-request with master.
Thank you!
oar/core/advisory.py
Outdated
| @@ -31,7 +31,7 @@ def get_advisories(self): | |||
| Get all advisories | |||
|
|
|||
| Returns: | |||
| []Advisory: all advisory wrappers | |||
| List[Advisory] : all advisory wrappers | |||
There was a problem hiding this comment.
The method returns a different type.
There was a problem hiding this comment.
To clarify it a little bit. It's not necessary wrong and it is only a documentation, so we potentially could use also the one you used. However, it's better to align the changes with the rest of the file and use standard built-in type.
oar/core/advisory.py
Outdated
| @@ -51,7 +51,7 @@ def get_jira_issues(self): | |||
| Get all jira issues from advisories in a release | |||
|
|
|||
| Returns: | |||
| []: all jira issues from advisories | |||
| List: all jira issues from advisories | |||
There was a problem hiding this comment.
The method returns a different type.
There was a problem hiding this comment.
To clarify it a little bit. It's not necessary wrong and it is only a documentation, so we potentially could use also the one you used. However, it's better to align the changes with the rest of the file and use standard built-in type.
oar/core/advisory.py
Outdated
| @@ -199,7 +199,7 @@ def drop_bugs(self): | |||
| AdvisoryException: error when dropping bugs from advisory | |||
|
|
|||
| Returns: | |||
| []: bugs cannot be dropped | |||
| Tuple [list, list]: bugst to be dropped, bugs cannot be dropped | |||
There was a problem hiding this comment.
The syntax is valid, but we should keep the types consistent across files. It's a good practice to use standard types.
The method returns a different type.
There was a problem hiding this comment.
There is also a typo at this line.
oar/core/advisory.py
Outdated
| @@ -252,7 +252,7 @@ def check_cve_tracker_bug(self): | |||
| AdvisoryException: error when invoke elliott cmd | |||
|
|
|||
| Returns: | |||
| json: missed CVE tracker bugs | |||
| List[str]: missed CVE tracker bugs | |||
There was a problem hiding this comment.
The method returns a different type.
There was a problem hiding this comment.
To clarify it a little bit. It's not necessary wrong and it is only a documentation, so we potentially could use also the one you used. However, it's better to align the changes with the rest of the file and use standard built-in type.
oar/core/jira.py
Outdated
|
|
||
| Returns: | ||
| []JiraIssue: jira subtask list | ||
| List [JiraIssue]: jira subtask list |
There was a problem hiding this comment.
The syntax is valid, but we should keep the types consistent across files. It's a good practice to use standard types.
The method returns a different type.
|
@ltroiano-rh one more note, to avoid having the same commit message in git history, use a different commit message. Please keep the jira id in it. |
|
This PR follows up #454 |
ltroiano-rh
force-pushed
the
ltroiano-release-test-changes
branch
from
dbbcd70 to
0600f60
Compare
openshift-merge-robot
removed
the
needs-rebase
There was a problem hiding this comment.
Hello @ltroiano-rh ,
There are still a few lines that need to be fixed.
The commit messages need to be adjusted. Don't use "Resolving conflicts" in the initial commit message.
Could you please also update pull-request name according to updated commit message?
Thanks!
oar/core/advisory.py
Outdated
| @@ -31,7 +31,7 @@ def get_advisories(self): | |||
| Get all advisories | |||
|
|
|||
| Returns: | |||
| Advisory (list): all advisory wrappers | |||
| list: all advisory wrappers | |||
There was a problem hiding this comment.
Part of the type is missing here.
oar/core/jira.py
Outdated
| @@ -43,7 +43,7 @@ def get_issue(self, key): | |||
| key (str): JIRA issue key | |||
|
|
|||
| Returns: | |||
| JiraIssue: object of JiraIssue | |||
| list: object of JiraIssue | |||
There was a problem hiding this comment.
The method returns a different type.
oar/core/jira.py
Outdated
|
|
||
| Returns: | ||
| JiraIssue (list): jira subtask list | ||
| list: jira subtask list |
There was a problem hiding this comment.
Part of the type is missing here.
ltroiano-rh
changed the title
ltroiano-rh
force-pushed
the
ltroiano-release-test-changes
branch
from
0600f60 to
a499cd5
Compare
There was a problem hiding this comment.
Hello @ltroiano-rh ,
please update your commit message. It's a docstring, not a dockstring. Also include more descriptive message that this is actually a followup pull-request. For example: OCPERT-56 OAR Correct wrong data types in docstring
You are also missing the changes you previously made.
Thanks!
| @@ -169,13 +169,11 @@ def change_advisory_status(self, target_status=AD_STATUS_REL_PREP): | |||
| Change advisories status, e.g. REL_PREP | |||
|
|
|||
| Args: | |||
| status (str, optional): status used to update. Defaults to REL_PREP. | |||
| target_status (str, optional): status used to update. Defaults to REL_PREP. | |||
|
|
|||
| Raises: | |||
| AdvisoryException: error when update advisory status | |||
|
|
|||
There was a problem hiding this comment.
Redundant new line here
openshift-ci
bot
added
the
do-not-merge/work-in-progress
OAR Fix wrong data types in docstring in https://github.com/openshift/release-tests repository.
Details:
OAR tool docstring use wrong data types in docstring. For example [] instead of list.
Update all docstrings in OAR tool to use correct OAR types.
Scope:
OAR tool only.
It is not a goal to provide new docstrings, or add types to actual code.