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.

Sign up for GitHub

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

Jump to bottom

csmock: export functions that can be reused by cspodman #115

Merged
merged 4 commits into from
Sep 22, 2023
Merged

csmock: export functions that can be reused by cspodman #115

merged 4 commits into from
Sep 22, 2023

Conversation

lbossis
Copy link
Contributor

@lbossis lbossis commented Aug 16, 2023

Ready for review

@lbossis lbossis marked this pull request as ready for review August 16, 2023 03:01
kdudka
kdudka reviewed Aug 16, 2023
View reviewed changes
Copy link
Member

@kdudka kdudka left a comment

Choose a reason for hiding this comment

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

Thank you for working on this. I am marking this pull request as a draft until the related change in cspodman is proven to work as expected.

py/common/util.py Outdated Show resolved Hide resolved
@kdudka kdudka marked this pull request as draft August 16, 2023 07:27
lzaoral
lzaoral reviewed Aug 16, 2023
View reviewed changes
py/csmock Outdated Show resolved Hide resolved
github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 16, 2023
View reviewed changes
py/common/util.py Fixed Show fixed Hide fixed
py/common/util.py Fixed Show fixed Hide fixed
py/common/util.py Fixed Show fixed Hide fixed
py/common/util.py Fixed Show fixed Hide fixed
py/common/util.py Fixed Show fixed Hide fixed
py/common/util.py Fixed Show fixed Hide fixed
py/common/util.py Fixed Show fixed Hide fixed
py/csmock Fixed Show fixed Hide fixed
github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 17, 2023
View reviewed changes
py/common/util.py Fixed Show fixed Hide fixed
py/common/util.py Fixed Show fixed Hide fixed
py/common/util.py Fixed Show fixed Hide fixed
@lbossis lbossis force-pushed the code-refactoring-move-function-handle_known_fp_list-to-csmock-common--OSH-151 branch from 44c6d5c to f115b7e Compare August 17, 2023 16:19
github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 17, 2023
View reviewed changes
py/common/util.py Fixed Show fixed Hide fixed
py/common/util.py Fixed Show fixed Hide fixed
@lbossis lbossis force-pushed the code-refactoring-move-function-handle_known_fp_list-to-csmock-common--OSH-151 branch from f115b7e to 687ac1d Compare August 17, 2023 17:02
github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 17, 2023
View reviewed changes
py/common/util.py Fixed Show fixed Hide fixed
@lbossis lbossis force-pushed the code-refactoring-move-function-handle_known_fp_list-to-csmock-common--OSH-151 branch from 687ac1d to c5df618 Compare August 17, 2023 18:13
@lbossis lbossis marked this pull request as ready for review August 17, 2023 18:46
@kdudka
Copy link
Member

kdudka commented Aug 22, 2023

@lbossis Thank you for working on this. I am marking this pull request as a draft until the related change in cspodman is proven to work as expected.

@kdudka kdudka marked this pull request as draft August 22, 2023 07:44
kdudka
kdudka reviewed Sep 13, 2023
View reviewed changes
py/common/util.py Outdated
in order to expose it for potential use where it might fit
"""
# install global filter of known false positives
filter_cmd = f"csdiff --json-output --show-internal \"{props.known_false_positives}\""
Copy link
Member

Choose a reason for hiding this comment

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

@lbossis This would break csmock because csdiff takes two operands and you omitted the trailing -:

csmock/py/csmock

Line 676 in 0671e2a

filter_cmd = 'csdiff --json-output --show-internal "%s" -' % props.known_false_positives

kdudka pushed a commit to lbossis/csmock that referenced this pull request Sep 13, 2023
@lbossis @kdudka
csmock: export handle_known_fp_list() and require_file()
4fe62b6 
... so that they can be reused by cspodman

Related: https://issues.redhat.com/browse/OSH-151
Closes: csutils#115
kdudka added a commit to lbossis/csmock that referenced this pull request Sep 13, 2023
@kdudka
csmock: update scan metadata in handle_known_fp_list()
019fd85 
... so that it can be reused by cspodman

Related: https://issues.redhat.com/browse/OSH-151
Closes: csutils#115
kdudka added a commit to lbossis/csmock that referenced this pull request Sep 13, 2023
@kdudka
csmock: export finalize_results()
8a0fd05 
... so that it can be reused by cspodman

Related: https://issues.redhat.com/browse/OSH-151
Closes: csutils#115
kdudka added a commit to lbossis/csmock that referenced this pull request Sep 13, 2023
@kdudka
csmock: export apply_result_filters()
dd5baab 
... so that it can be reused by cspodman

Related: https://issues.redhat.com/browse/OSH-151
Closes: csutils#115
@kdudka kdudka force-pushed the code-refactoring-move-function-handle_known_fp_list-to-csmock-common--OSH-151 branch from a4f417b to dd5baab Compare September 13, 2023 15:22
@kdudka kdudka changed the title Code refactoring to enable reuse of the function handle_known_fp_list csmock: export functions that can be reused by cspodman Sep 13, 2023
github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 13, 2023
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

vcs-diff-lint found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

@kdudka kdudka requested review from kdudka and removed request for kdudka September 13, 2023 15:23
@kdudka kdudka marked this pull request as ready for review September 13, 2023 15:24
kdudka added a commit to lbossis/csmock that referenced this pull request Sep 13, 2023
@kdudka
csmock: export finalize_results()
b14c9ee 
... so that it can be reused by cspodman

Related: https://issues.redhat.com/browse/OSH-151
Closes: csutils#115
kdudka added a commit to lbossis/csmock that referenced this pull request Sep 13, 2023
@kdudka
csmock: export apply_result_filters()
e109338 
... so that it can be reused by cspodman

Related: https://issues.redhat.com/browse/OSH-151
Closes: csutils#115
@kdudka kdudka force-pushed the code-refactoring-move-function-handle_known_fp_list-to-csmock-common--OSH-151 branch from dd5baab to e109338 Compare September 13, 2023 15:27
github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 13, 2023
View reviewed changes
py/common/results.py Dismissed Show dismissed Hide dismissed
py/common/results.py Dismissed Show dismissed Hide dismissed
py/common/results.py Dismissed Show dismissed Hide dismissed
py/common/results.py Dismissed Show dismissed Hide dismissed
py/common/results.py Dismissed Show dismissed Hide dismissed
py/common/results.py Dismissed Show dismissed Hide dismissed
py/common/results.py Dismissed Show dismissed Hide dismissed
py/common/results.py Dismissed Show dismissed Hide dismissed
py/csmock Dismissed Show dismissed Hide dismissed
py/csmock Dismissed Show dismissed Hide dismissed
@kdudka kdudka requested a review from lzaoral September 13, 2023 15:29
@kdudka kdudka requested review from kdudka and removed request for kdudka September 13, 2023 15:29
@lzaoral lzaoral removed their request for review September 14, 2023 10:47
lzaoral

This comment was marked as off-topic.

Sign in to view

@kdudka kdudka requested a review from rhyw September 18, 2023 09:47
@rhyw
Copy link
Contributor

rhyw commented Sep 21, 2023

Didn't verify but the proposed changes LGTM.

@kdudka kdudka force-pushed the code-refactoring-move-function-handle_known_fp_list-to-csmock-common--OSH-151 branch from e109338 to 40b402c Compare September 22, 2023 10:28
@kdudka
Copy link
Member

kdudka commented Sep 22, 2023

@rhyw Thanks for review!

@kdudka kdudka closed this in e570654 Sep 22, 2023
@kdudka kdudka merged commit 40b402c into csutils:main Sep 22, 2023
3 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kdudka kdudka kdudka left review comments

@rhyw rhyw rhyw approved these changes

@lzaoral lzaoral lzaoral left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lbossis @kdudka @rhyw @lzaoral