-
Notifications
You must be signed in to change notification settings - Fork 379
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
Local compare mode takes in-code-suppressions into consideration. #858
Local compare mode takes in-code-suppressions into consideration. #858
Conversation
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.
Style issues only.
@@ -21,6 +21,8 @@ | |||
from libcodechecker.logger import LoggerFactory | |||
from libcodechecker.output_formatters import twodim_to_str | |||
from libcodechecker.report import Report | |||
from libcodechecker import suppress_handler |
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.
Import order is not proper here. from libcodechecker import X
must precede from libcodecher.a import b
.
tests/functional/diff/__init__.py
Outdated
@@ -16,6 +16,7 @@ | |||
import sys | |||
import time | |||
import uuid | |||
from distutils import dir_util |
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.
Import order is not proper here. Not in this line, and not the line directly below.
tests/functional/diff/__init__.py
Outdated
@@ -45,6 +55,10 @@ def setup_package(): | |||
test_project = 'cpp' | |||
|
|||
test_project_path = project.path(test_project) | |||
test_project_path_altered = os.path.join(TEST_WORKSPACE, "cpp_copy") | |||
# we create a copy of the test project which we will change |
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 is not a full sentence.
@@ -16,6 +16,7 @@ void f(struct S s); | |||
|
|||
void test1() { | |||
struct S s; | |||
//insert_suppress_here |
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.
Can we put a space between //
and the keyword?
3d12010
to
97c6b39
Compare
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.
Generally LGTM! But is it a good idea to have tests alter the files instead of having a separate file with the suppression, or always having the suppression in the file?
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.
Please resolve the merge conflicts.
97c6b39
to
26fbe39
Compare
12e3eaf
to
2b4a2be
Compare
2b4a2be
to
b599589
Compare
This fixes #851