-
Notifications
You must be signed in to change notification settings - Fork 579
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
checkstyle #992
checkstyle #992
Conversation
not fully tested yet |
Thanks for your contribution! Reviewing pull requests take really a lot of time and we're all volunteers. Please make sure you go through the following check list and complete them all before pinging someone for a review.
As you learn things over your Pull Request please help others on the chat and on PRs to get their stuff right as well! |
|
||
def run(self, filename, file, dependency_results, | ||
preferred_quotation: str='"'): | ||
if isinstance(dependency_results[AnnotationBear.name][0].contents, str): |
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.
Line is longer than allowed. (80 > 79)
LineLengthBear, severity NORMAL, section linelength
.
diff.change_line(sourcerange.start.line, | ||
file[sourcerange.start.line - 1], | ||
replacement) | ||
yield Result(self, "Your quotation sucks!", diff.affected_code(filename), diffs={filename: diff}) |
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.
Line is longer than allowed. (109 > 79)
LineLengthBear, severity NORMAL, section linelength
.
before = file[sourcerange.start.line - 1][:sourcerange.start.column-1] | ||
after = file[sourcerange.start.line - 1][sourcerange.end.column:] | ||
|
||
replacement = before + preferred_quotation + str_contents + preferred_quotation + after |
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.
Line is longer than allowed. (95 > 79)
LineLengthBear, severity NORMAL, section linelength
.
before = file[sourcerange.start.line - 1][:sourcerange.start.column-1] | ||
after = file[sourcerange.start.line - 1][sourcerange.end.column:] | ||
|
||
replacement = before + preferred_quotation + str_contents + preferred_quotation + after |
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.
The code does not comply to PEP8.
PEP8Bear, severity NORMAL, section autopep8
.
The issue can be fixed by applying the following patch:
--- a/bears/general/QuotesBear.py
+++ b/bears/general/QuotesBear.py
@@ -20,7 +20,8 @@
before = file[sourcerange.start.line - 1][:sourcerange.start.column-1]
after = file[sourcerange.start.line - 1][sourcerange.end.column:]
- replacement = before + preferred_quotation + str_contents + preferred_quotation + after
+ replacement = before + preferred_quotation + \
+ str_contents + preferred_quotation + after
if replacement != file[sourcerange.start.line - 1]:
diff = Diff(file)
diff.change_line(sourcerange.start.line,
:param preferred_quotation: Your preferred quotation character, e.g. | ||
``"`` or ``'``. | ||
""" | ||
if isinstance(dependency_results[AnnotationBear.name][0].contents, str): |
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.
Line is longer than allowed. (80 > 79)
LineLengthBear, severity NORMAL, section linelength
.
if not isinstance(dependency_results[AnnotationBear.name][0], | ||
HiddenResult): | ||
return | ||
if isinstance(dependency_results[AnnotationBear.name][0].contents, str): |
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.
Line is longer than allowed. (80 > 79)
LineLengthBear, severity NORMAL, section linelength
.
ack fac086e hope it still works :P |
a tuple of SourceRanges of comments | ||
respectively. | ||
:param language: | ||
Language to be whose annotations are to be searched. |
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.
Doesn't make complete sense, Incorrect grammer?
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.
filed a newcomer issue, this is just reformatted and not changed. #994
AUTHORS_EMAILS = {'[email protected]'} | ||
LICENSE = 'AGPL-3.0' | ||
CAN_DETECT = {'Formatting'} | ||
|
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.
ASCIINEMA ?
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 was too lazy and they make good newcomer issues :P
reack a4dd7a2 |
@@ -94,4 +94,5 @@ def create_arguments(filename, file, config_file, | |||
"java-unusedcode": not allow_unused_code} | |||
rules = ','.join(key for key in options if options[key]) | |||
|
|||
return which("run.sh"), "pmd", "-R", rules, "-d", filename | |||
executable = which("pmd") or which("run.sh") # On mac it'll be PMD e.g. |
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.
The code does not comply to PEP8.
PEP8Bear, severity NORMAL, section autopep8
.
The issue can be fixed by applying the following patch:
--- a/bears/java/JavaPMDBear.py
+++ b/bears/java/JavaPMDBear.py
@@ -94,5 +94,6 @@
"java-unusedcode": not allow_unused_code}
rules = ','.join(key for key in options if options[key])
- executable = which("pmd") or which("run.sh") # On mac it'll be PMD e.g.
+ # On mac it'll be PMD e.g.
+ executable = which("pmd") or which("run.sh")
return executable, "pmd", "-R", rules, "-d", filename
@@ -94,4 +94,5 @@ def create_arguments(filename, file, config_file, | |||
"java-unusedcode": not allow_unused_code} | |||
rules = ','.join(key for key in options if options[key]) | |||
|
|||
return which("run.sh"), "pmd", "-R", rules, "-d", filename | |||
executable = which("pmd") or which("run.sh") # On mac it'll be PMD e.g. |
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.
Line is longer than allowed. (80 > 79)
LineLengthBear, severity NORMAL, section linelength
.
af58a7b
to
3820ffe
Compare
"checkstyle.jar") | ||
'http://sourceforge.net/projects/checkstyle/files/checkstyle/7.2' | ||
'/checkstyle-7.2-all.jar', | ||
"checkstyle-7-2.jar") |
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 still dont fix 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.
seems not :/
@Adrianzatreanu can you debug this on windows? |
"google": "https://raw.githubusercontent.com/checkstyle/checkstyle/master/src/main/resources/google_checks.xml", | ||
"sun": 'https://raw.githubusercontent.com/checkstyle/checkstyle/master/src/main/resources/sun_checks.xml', | ||
"google": "https://raw.githubusercontent.com/checkstyle/checkstyle/checkstyle-7.2/src/main/resources/google_checks.xml", | ||
"sun": 'https://raw.githubusercontent.com/checkstyle/checkstyle/master/src/checkstyle-7.2/resources/sun_checks.xml', |
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.
master/src/checkstyle-7.2
??
-> checkstyle-7.2/src/main
?
unack 3820ffe |
yeah, not fixing it yet :/ |
Those file may have been updated
@Makman2 can you test run this on windows? |
I believe this is the bug you encountered, and are fixing: Checkstyle with style google fails with CheckstyleException ? |
@jayvdb had a way better fix |
Bumping the version to 7.2 would still be useful. |
No description provided.