-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 1) Col: 1 E265 block comment should start with '# '
-
bot/reviewbot/tools/shellcheck.py (Diff revision 1) Col: 1 E265 block comment should start with '# '
-
-
-
-
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 1) Col: 2 E265 block comment should start with '# '
-
-
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 1) Col: 1 E101 indentation contains mixed spaces and tabs
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 1) Col: 9 E265 block comment should start with '# '
-
-
-
-
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 1) Col: 80 E501 line too long (107 > 79 characters)
-
-
-
-
-
Write a ReviewBot plugin for ShellCheck
Review Request #7701 — Created Oct. 15, 2015 and discarded
Information | |
---|---|
xutong | |
ReviewBot | |
release-0.2.x | |
Reviewers | |
reviewbot | |
Pick a static analysis tool, shellcheck, and write the plugin.
Shellcheck is a static analysis tool, like pep8, that can automatically detect problem with sh/bash script and commands.
The feature of the shellcheck plugin will let ReviewBot can prove feedback for sh/bash script. Right now, reviewboard can detect problem with sh/bash script, if sh/bash is included in a new review request.
- Test shellcheck plugin:
1). Create a Reviewbot user, that it will review sh/bash script.
2). Make own repository, add it to RB, and add a .reviewboardrc with a REVIEWBOARD_URL pointing to localhost.
3). Set reviewboard extention so that reviewbot tool shellcheck will run aotumatically when there is a new review request with sh/bash script.
4). Create a review request on local instance of Review Board with sh/bash script. Once the ReviewBot user make a review of the code, compare the result with running shellcheck in terminal on the same sh/bask script. If all the outputs are same, shellcheck will works fine.
5). After all above test are correct. Test explicitly exclude the specified codes from the report. Compare with the original shellcheck in terminal, and see if all the output are correct.- Run unit tests on reviewboard, see if the new plugin break any other features of reviewboard.
Description | From | Last Updated |
---|---|---|
Col: 1 E265 block comment should start with '# ' |
![]() |
|
Col: 1 E265 block comment should start with '# ' |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 2 E265 block comment should start with '# ' |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 E101 indentation contains mixed spaces and tabs |
![]() |
|
Col: 9 E112 expected an indented block |
![]() |
|
Col: 1 W293 blank line contains whitespace |
![]() |
|
Col: 9 E265 block comment should start with '# ' |
![]() |
|
Col: 1 W293 blank line contains whitespace |
![]() |
|
Col: 80 E501 line too long (85 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (91 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (95 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (89 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (82 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (107 > 79 characters) |
![]() |
|
Col: 25 W291 trailing whitespace |
![]() |
|
Col: 57 W291 trailing whitespace |
![]() |
|
Col: 1 W293 blank line contains whitespace |
![]() |
|
Col: 1 W293 blank line contains whitespace |
![]() |
|
Col: 20 W291 trailing whitespace |
![]() |
|
Col: 1 E265 block comment should start with '# ' |
![]() |
|
Col: 1 E265 block comment should start with '# ' |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 2 E265 block comment should start with '# ' |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 E101 indentation contains mixed spaces and tabs |
![]() |
|
Col: 9 E112 expected an indented block |
![]() |
|
Col: 9 E265 block comment should start with '# ' |
![]() |
|
Col: 80 E501 line too long (85 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (91 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (95 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (89 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (82 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (107 > 79 characters) |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 2 E303 too many blank lines (2) |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 E101 indentation contains mixed spaces and tabs |
![]() |
|
Col: 9 E112 expected an indented block |
![]() |
|
Col: 9 E265 block comment should start with '# ' |
![]() |
|
Col: 20 W292 no newline at end of file |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 2 E303 too many blank lines (2) |
![]() |
|
Col: 2 E113 unexpected indentation |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 E101 indentation contains mixed spaces and tabs |
![]() |
|
Col: 9 E112 expected an indented block |
![]() |
|
Col: 1 E101 indentation contains mixed spaces and tabs |
![]() |
|
Col: 1 E101 indentation contains mixed spaces and tabs |
![]() |
|
Col: 1 W191 indentation contains tabs |
![]() |
|
Col: 1 E101 indentation contains mixed spaces and tabs |
![]() |
|
Col: 20 W292 no newline at end of file |
![]() |
|
Col: 1 E302 expected 2 blank lines, found 1 |
![]() |
|
Col: 5 E112 expected an indented block |
![]() |
|
Needs a docstring. |
|
|
Let's start the versioning at 0.1 instead of 0.4.1, since this is the first version of the ShellCheck tool. |
|
|
"script" Missing period. |
|
|
Remove this comment since the code is self-explanatory. |
|
|
I thought we talked about getting the JSON output format so that we could parse it more easily and robustly … |
|
|
Make sure these gracefully handle unexpected output. Also, variable names must always be in "this_case" and not "ThisCase". |
|
|
You can use python tuple unpacking to make this much nicer: line_num, col_num, msg = line.split(':', 3) As Christian said, … |
|
|
Col: 67 W291 trailing whitespace |
![]() |
|
Col: 13 E265 block comment should start with '# ' |
![]() |
|
Col: 13 E265 block comment should start with '# ' |
![]() |
|
Col: 13 E265 block comment should start with '# ' |
![]() |
|
Col: 13 E265 block comment should start with '# ' |
![]() |
|
Col: 70 W291 trailing whitespace |
![]() |
|
Col: 67 W291 trailing whitespace |
![]() |
|
Col: 13 E265 block comment should start with '# ' |
![]() |
|
Col: 13 E265 block comment should start with '# ' |
![]() |
|
Col: 13 E265 block comment should start with '# ' |
![]() |
|
Col: 13 E265 block comment should start with '# ' |
![]() |
|
Col: 70 W291 trailing whitespace |
![]() |
|
Col: 1 W293 blank line contains whitespace |
![]() |
|
Col: 61 W291 trailing whitespace |
![]() |
|
Can you remove this, since it doesn't really add any valuable infomation for the class? |
|
|
s/Ruturn/Return |
|
|
Add a blank line before this line. |
|
|
Variables should use lowercase words separated by underscores (e.g. Line_Num would be line_num instead). |
|
|
'os' imported but unused |
![]() |
|
'make_tempfile' imported but unused |
![]() |
|
Col: 13 E265 block comment should start with '# ' |
![]() |
|
This has been mentioned before, but variable names should be lower-case, separated by underscores (e.g. file_name instead of File_name). |
|
|
Col: 14 E111 indentation is not a multiple of four |
![]() |
|
Col: 14 E111 indentation is not a multiple of four |
![]() |
|
'os' imported but unused |
![]() |
|
'make_tempfile' imported but unused |
![]() |
|
Col: 13 E265 block comment should start with '# ' |
![]() |
|
Col: 1 W293 blank line contains whitespace |
![]() |
|
Col: 14 E111 indentation is not a multiple of four |
![]() |
|
Col: 14 E111 indentation is not a multiple of four |
![]() |
|
Col: 54 E201 whitespace after '(' |
![]() |
|
Col: 17 E128 continuation line under-indented for visual indent |
![]() |
|
'os' imported but unused |
![]() |
|
'make_tempfile' imported but unused |
![]() |
|
Col: 13 E265 block comment should start with '# ' |
![]() |
|
Col: 1 W293 blank line contains whitespace |
![]() |
|
Col: 14 E111 indentation is not a multiple of four |
![]() |
|
Col: 14 E111 indentation is not a multiple of four |
![]() |
|
Col: 54 E201 whitespace after '(' |
![]() |
|
Col: 17 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 1 W293 blank line contains whitespace |
![]() |
|
Col: 14 E111 indentation is not a multiple of four |
![]() |
|
Col: 14 E111 indentation is not a multiple of four |
![]() |
|
Couple of things here: - The first line of this docstring should describe what the class is, instead of stating … |
|
|
This does not return anything. It is a class. |
|
|
Remove this line. |
|
|
Since ShellCheck also identifies potential failures in addition to just style errors, let's update the description to: Checks shell scripts … |
|
|
Are there any options we might want to add? --exclude/-e looks pretty useful at least: http://manpages.ubuntu.com/manpages/utopic/man1/shellcheck.1.html |
|
|
Comments should be written in full sentences, like: # Ignore the file if it is not a shell script. |
|
|
I think the code is pretty self-explanatory, so let's remove these comments? |
|
|
It'd be useful to add a comment above this line showing an example of shellcheck's output. |
|
|
These lines are over-indented by 1 space. Can we also rename mesg to msg? |
|
|
Col: 14 E111 indentation is not a multiple of four |
![]() |
|
There's no need to make col_num an int here, so we can remove that. |
|
|
Col: 14 E111 indentation is not a multiple of four |
![]() |
|
Blank line between these. |
|
|
Col: 80 E501 line too long (94 > 79 characters) |
![]() |
|
This string should end with a space. |
|
|
Col: 82 W291 trailing whitespace |
![]() |
|
Col: 80 E501 line too long (81 > 79 characters) |
![]() |
|
Likewise. |
|
|
Col: 83 W291 trailing whitespace |
![]() |
|
Col: 80 E501 line too long (82 > 79 characters) |
![]() |
|
But this one should not. |
|
|
Col: 80 E501 line too long (91 > 79 characters) |
![]() |
|
This should be a ChoiceField, not a CharField |
|
|
Col: 80 E501 line too long (86 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (81 > 79 characters) |
![]() |
|
Col: 34 W291 trailing whitespace |
![]() |
|
Remove this comment. |
|
|
This would be good in the docstring of the function. |
|
|
This can all be on one line. |
|
|
Col: 14 E111 indentation is not a multiple of four |
![]() |
|
Col: 14 E111 indentation is not a multiple of four |
![]() |
|
int(x) can fail if x is invalid input. This should have a try..except around it. |
|
|
Col: 65 W291 trailing whitespace |
![]() |
|
Col: 1 W293 blank line contains whitespace |
![]() |
|
Col: 19 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 35 W291 trailing whitespace |
![]() |
|
Col: 32 E225 missing whitespace around operator |
![]() |
|
Col: 33 E225 missing whitespace around operator |
![]() |
|
Col: 14 E111 indentation is not a multiple of four |
![]() |
|
Col: 14 E111 indentation is not a multiple of four |
![]() |
|
Col: 19 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 14 E111 indentation is not a multiple of four |
![]() |
|
Col: 14 E111 indentation is not a multiple of four |
![]() |
|
Col: 19 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 44 W291 trailing whitespace |
![]() |
|
Col: 46 W291 trailing whitespace |
![]() |
|
Col: 17 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 49 W291 trailing whitespace |
![]() |
|
Col: 17 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 13 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 13 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 19 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 19 E124 closing bracket does not match visual indentation |
![]() |
|
Col: 17 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 17 E124 closing bracket does not match visual indentation |
![]() |
|
Col: 17 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 17 E124 closing bracket does not match visual indentation |
![]() |
|
Col: 13 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 13 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 13 E124 closing bracket does not match visual indentation |
![]() |
|
Col: 19 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 19 E124 closing bracket does not match visual indentation |
![]() |
|
Col: 17 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 17 E124 closing bracket does not match visual indentation |
![]() |
|
Col: 17 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 17 E124 closing bracket does not match visual indentation |
![]() |
|
Col: 13 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 13 E128 continuation line under-indented for visual indent |
![]() |
|
Col: 19 E126 continuation line over-indented for hanging indent |
![]() |
|
Col: 19 E123 closing bracket does not match indentation of opening bracket's line |
![]() |
|
Col: 17 E123 closing bracket does not match indentation of opening bracket's line |
![]() |
|
Col: 17 E123 closing bracket does not match indentation of opening bracket's line |
![]() |
|
Col: 13 E123 closing bracket does not match indentation of opening bracket's line |
![]() |
|
Col: 19 E126 continuation line over-indented for hanging indent |
![]() |
|
Col: 19 E123 closing bracket does not match indentation of opening bracket's line |
![]() |
|
Col: 32 E126 continuation line over-indented for hanging indent |
![]() |
|
Col: 32 E126 continuation line over-indented for hanging indent |
![]() |
|
Col: 24 E126 continuation line over-indented for hanging indent |
![]() |
|
Col: 19 E128 continuation line under-indented for visual indent |
![]() |
|
This doesn't belong in the docstring of the tool. |
|
|
Col: 19 E126 continuation line over-indented for hanging indent |
![]() |
|
These lines should be indented 4 relative to description. |
|
|
This won't work as an IntegerField. This will have to be a CharField or similar. |
|
|
Should only be one space after "Diagnostic". Also, what if we want to ignore more than one error? This should … |
|
|
This would be a good place to indicate the format of shellcheck output. |
|
|
Are we absolutely certain that we don't want to get JSON output from the shellcheck tool, parse it, and iterate … |
|
|
Re: above comments, this should check for a comma-separated list in this setting and loop through them. Shellcheck may allow … |
|
|
Format this as: output = execute(command_line, split_lines=True, ignore_errors=True) |
|
|
Blank line between these. |
|
|
This error message is useless. Also, you don't want to print (unless thats what other tools do for error logging). |
|
|
Col: 19 E126 continuation line over-indented for hanging indent |
![]() |
|
Col: 26 E201 whitespace after '(' |
![]() |
|
Col: 19 E126 continuation line over-indented for hanging indent |
![]() |
|
Col: 80 E501 line too long (80 > 79 characters) |
![]() |
|
Col: 52 W291 trailing whitespace |
![]() |
|
Col: 80 E501 line too long (80 > 79 characters) |
![]() |
|
Col: 52 W291 trailing whitespace |
![]() |
|
Col: 30 W291 trailing whitespace |
![]() |
|
Col: 34 W291 trailing whitespace |
![]() |
|
Standard library imports should come first, then a blank line, then package-local imports. |
|
|
There are way too many spaces in here. One space between words, one space between sentences. |
|
|
This should probably be a comment, not a docstring. |
|
|
Can we reformat this as such: command_line.append('--format=%s' % self.settings['specify_dialect']) |
|
|
Same here. |
|
|
We should include the .split() call inside the try block, and catch IndexError |
|
|
This is a very poor error message. |
|
|
Col: 50 W291 trailing whitespace |
![]() |
|
Col: 38 E127 continuation line over-indented for visual indent |
![]() |
|
Col: 51 W291 trailing whitespace |
![]() |
|
Col: 38 E127 continuation line over-indented for visual indent |
![]() |
|
Col: 52 W292 no newline at end of file |
![]() |
|
Col: 50 W291 trailing whitespace |
![]() |
|
Col: 38 E127 continuation line over-indented for visual indent |
![]() |
|
Col: 51 W291 trailing whitespace |
![]() |
|
Col: 38 E127 continuation line over-indented for visual indent |
![]() |
|
Col: 52 W292 no newline at end of file |
![]() |
|
You should mention this accepts comma separated list. |
|
|
This is not useful. Remove the try: block. |
|
|
Col: 20 W292 no newline at end of file |
![]() |


-
Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py
-
bot/reviewbot/tools/shellcheck.py (Diff revision 2) Col: 1 E265 block comment should start with '# '
-
bot/reviewbot/tools/shellcheck.py (Diff revision 2) Col: 1 E265 block comment should start with '# '
-
-
-
-
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 2) Col: 2 E265 block comment should start with '# '
-
-
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 2) Col: 1 E101 indentation contains mixed spaces and tabs
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 2) Col: 9 E265 block comment should start with '# '
-
-
-
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 2) Col: 80 E501 line too long (107 > 79 characters)

-
Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py
-
-
-
-
-
-
-
-
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 3) Col: 1 E101 indentation contains mixed spaces and tabs
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 3) Col: 9 E265 block comment should start with '# '
-

-
Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py
-
-
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 4) Col: 1 E101 indentation contains mixed spaces and tabs
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 4) Col: 1 E101 indentation contains mixed spaces and tabs
-
bot/reviewbot/tools/shellcheck.py (Diff revision 4) Col: 1 E101 indentation contains mixed spaces and tabs
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 4) Col: 1 E101 indentation contains mixed spaces and tabs
-

-
Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py
-
-

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 6) I thought we talked about getting the JSON output format so that we could parse it more easily and robustly than we're doing on lines 43-48?
-
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 6) Make sure these gracefully handle unexpected output.
Also, variable names must always be in "this_case" and not "ThisCase".
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 6) You can use python tuple unpacking to make this much nicer:
line_num, col_num, msg = line.split(':', 3)
As Christian said, make sure to surround this in
try
/except
to handle unexpected input.
-
Looks like a good start!
The workers find installed tools through entry points, so you'll need to add a new entry-point for ShellCheck in
bot/setup.py
. -
bot/reviewbot/tools/shellcheck.py (Diff revision 6) Let's start the versioning at 0.1 instead of 0.4.1, since this is the first version of the ShellCheck tool.
-
bot/reviewbot/tools/shellcheck.py (Diff revision 6) Remove this comment since the code is self-explanatory.

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 7) Col: 13 E265 block comment should start with '# '
-
bot/reviewbot/tools/shellcheck.py (Diff revision 7) Col: 13 E265 block comment should start with '# '
-
bot/reviewbot/tools/shellcheck.py (Diff revision 7) Col: 13 E265 block comment should start with '# '
-
bot/reviewbot/tools/shellcheck.py (Diff revision 7) Col: 13 E265 block comment should start with '# '
-

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 8) Col: 13 E265 block comment should start with '# '
-
bot/reviewbot/tools/shellcheck.py (Diff revision 8) Col: 13 E265 block comment should start with '# '
-
bot/reviewbot/tools/shellcheck.py (Diff revision 8) Col: 13 E265 block comment should start with '# '
-
bot/reviewbot/tools/shellcheck.py (Diff revision 8) Col: 13 E265 block comment should start with '# '
-

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
-

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
Looks like you should be ready to test your shellcheck tool this week!
-
bot/reviewbot/tools/shellcheck.py (Diff revision 10) Can you remove this, since it doesn't really add any valuable infomation for the class?
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 10) Variables should use lowercase words separated by underscores (e.g. Line_Num would be line_num instead).

-
Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/reviewbot/tools/buildbot.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/reviewbot/tools/buildbot.py bot/setup.py
-
-
-
bot/reviewbot/tools/buildbot.py (Diff revision 12) Col: 13 E265 block comment should start with '# '
-
bot/reviewbot/tools/shellcheck.py (Diff revision 12) Col: 14 E111 indentation is not a multiple of four
-
bot/reviewbot/tools/shellcheck.py (Diff revision 12) Col: 14 E111 indentation is not a multiple of four
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 12) This has been mentioned before, but variable names should be lower-case, separated by underscores (e.g.
file_name
instead ofFile_name
).

-
Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/reviewbot/tools/buildbot.py bot/setup.py Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/reviewbot/tools/buildbot.py bot/setup.py
-
-
-
bot/reviewbot/tools/buildbot.py (Diff revision 13) Col: 13 E265 block comment should start with '# '
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 13) Col: 14 E111 indentation is not a multiple of four
-
bot/reviewbot/tools/shellcheck.py (Diff revision 13) Col: 14 E111 indentation is not a multiple of four
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 13) Col: 17 E128 continuation line under-indented for visual indent

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/reviewbot/tools/buildbot.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/reviewbot/tools/buildbot.py bot/setup.py
-
-
-
bot/reviewbot/tools/buildbot.py (Diff revision 14) Col: 13 E265 block comment should start with '# '
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 14) Col: 14 E111 indentation is not a multiple of four
-
bot/reviewbot/tools/shellcheck.py (Diff revision 14) Col: 14 E111 indentation is not a multiple of four
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 14) Col: 17 E128 continuation line under-indented for visual indent

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 15) Col: 14 E111 indentation is not a multiple of four
-
bot/reviewbot/tools/shellcheck.py (Diff revision 15) Col: 14 E111 indentation is not a multiple of four
Summary: |
|
||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
||||||||||||
Testing Done: |
|

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
bot/reviewbot/tools/shellcheck.py (Diff revision 16) Col: 14 E111 indentation is not a multiple of four
-
bot/reviewbot/tools/shellcheck.py (Diff revision 16) Col: 14 E111 indentation is not a multiple of four
-
Good work on this! I mostly have style comments.
Can you update your summary to "Add ShellCheck tool plugin.", and remove the first line of your description ("Pick a static analysis tool, shellcheck, and write the plugin.")? Also, can you mark David's issue as being fixed if you've already addressed it?
-
bot/reviewbot/tools/shellcheck.py (Diff revision 16) Couple of things here:
- The first line of this docstring should describe what the class is, instead of stating what the docstring is going to describe.
- Let's get rid of the return section since a class doesn't actually return anything (that section would make more sense as the docstring for thehandle_file
method).I think the docstring for this can be pretty simple, really (maybe just
"""ShellCheck tool plugin implementation."""
). -
-
bot/reviewbot/tools/shellcheck.py (Diff revision 16) Since ShellCheck also identifies potential failures in addition to just style errors, let's update the description to:
Checks shell scripts for style errors and potential bugs using ShellCheck.
-
bot/reviewbot/tools/shellcheck.py (Diff revision 16) Are there any options we might want to add?
--exclude
/-e
looks pretty useful at least: http://manpages.ubuntu.com/manpages/utopic/man1/shellcheck.1.html -
bot/reviewbot/tools/shellcheck.py (Diff revision 16) Comments should be written in full sentences, like:
# Ignore the file if it is not a shell script.
-
bot/reviewbot/tools/shellcheck.py (Diff revision 16) I think the code is pretty self-explanatory, so let's remove these comments?
-
bot/reviewbot/tools/shellcheck.py (Diff revision 16) It'd be useful to add a comment above this line showing an example of shellcheck's output.
-
bot/reviewbot/tools/shellcheck.py (Diff revision 16) These lines are over-indented by 1 space. Can we also rename
mesg
tomsg
? -
bot/reviewbot/tools/shellcheck.py (Diff revision 16) There's no need to make col_num an int here, so we can remove that.
Testing Done: |
|
||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 17 (+67) |

-
Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
bot/reviewbot/tools/shellcheck.py (Diff revision 17) Col: 80 E501 line too long (94 > 79 characters)
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 17) Col: 80 E501 line too long (81 > 79 characters)
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 17) Col: 80 E501 line too long (82 > 79 characters)
-
bot/reviewbot/tools/shellcheck.py (Diff revision 17) Col: 80 E501 line too long (91 > 79 characters)
-
bot/reviewbot/tools/shellcheck.py (Diff revision 17) Col: 80 E501 line too long (86 > 79 characters)
-
bot/reviewbot/tools/shellcheck.py (Diff revision 17) Col: 80 E501 line too long (81 > 79 characters)
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 17) Col: 14 E111 indentation is not a multiple of four
-
bot/reviewbot/tools/shellcheck.py (Diff revision 17) Col: 14 E111 indentation is not a multiple of four
-
-
-
-
-
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 17) This would be good in the docstring of the function.
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 17) int(x)
can fail ifx
is invalid input. This should have atry..except
around it.

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 18) Col: 19 E128 continuation line under-indented for visual indent
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 18) Col: 32 E225 missing whitespace around operator
-
bot/reviewbot/tools/shellcheck.py (Diff revision 18) Col: 33 E225 missing whitespace around operator
-
bot/reviewbot/tools/shellcheck.py (Diff revision 18) Col: 14 E111 indentation is not a multiple of four
-
bot/reviewbot/tools/shellcheck.py (Diff revision 18) Col: 14 E111 indentation is not a multiple of four

-
Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
bot/reviewbot/tools/shellcheck.py (Diff revision 19) Col: 19 E128 continuation line under-indented for visual indent
-
bot/reviewbot/tools/shellcheck.py (Diff revision 19) Col: 14 E111 indentation is not a multiple of four
-
bot/reviewbot/tools/shellcheck.py (Diff revision 19) Col: 14 E111 indentation is not a multiple of four

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
bot/reviewbot/tools/shellcheck.py (Diff revision 20) Col: 19 E128 continuation line under-indented for visual indent
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 20) Col: 17 E128 continuation line under-indented for visual indent
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 20) Col: 17 E128 continuation line under-indented for visual indent
-
bot/reviewbot/tools/shellcheck.py (Diff revision 20) Col: 13 E128 continuation line under-indented for visual indent
-
bot/reviewbot/tools/shellcheck.py (Diff revision 20) Col: 13 E128 continuation line under-indented for visual indent

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
bot/reviewbot/tools/shellcheck.py (Diff revision 21) Col: 19 E128 continuation line under-indented for visual indent
-
bot/reviewbot/tools/shellcheck.py (Diff revision 21) Col: 19 E124 closing bracket does not match visual indentation
-
bot/reviewbot/tools/shellcheck.py (Diff revision 21) Col: 17 E128 continuation line under-indented for visual indent
-
bot/reviewbot/tools/shellcheck.py (Diff revision 21) Col: 17 E124 closing bracket does not match visual indentation
-
bot/reviewbot/tools/shellcheck.py (Diff revision 21) Col: 17 E128 continuation line under-indented for visual indent
-
bot/reviewbot/tools/shellcheck.py (Diff revision 21) Col: 17 E124 closing bracket does not match visual indentation
-
bot/reviewbot/tools/shellcheck.py (Diff revision 21) Col: 13 E128 continuation line under-indented for visual indent
-
bot/reviewbot/tools/shellcheck.py (Diff revision 21) Col: 13 E128 continuation line under-indented for visual indent
-
bot/reviewbot/tools/shellcheck.py (Diff revision 21) Col: 13 E124 closing bracket does not match visual indentation
Testing Done: |
|
---|

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
bot/reviewbot/tools/shellcheck.py (Diff revision 22) Col: 19 E128 continuation line under-indented for visual indent
-
bot/reviewbot/tools/shellcheck.py (Diff revision 22) Col: 19 E124 closing bracket does not match visual indentation
-
bot/reviewbot/tools/shellcheck.py (Diff revision 22) Col: 17 E128 continuation line under-indented for visual indent
-
bot/reviewbot/tools/shellcheck.py (Diff revision 22) Col: 17 E124 closing bracket does not match visual indentation
-
bot/reviewbot/tools/shellcheck.py (Diff revision 22) Col: 17 E128 continuation line under-indented for visual indent
-
bot/reviewbot/tools/shellcheck.py (Diff revision 22) Col: 17 E124 closing bracket does not match visual indentation
-
bot/reviewbot/tools/shellcheck.py (Diff revision 22) Col: 13 E128 continuation line under-indented for visual indent
-
bot/reviewbot/tools/shellcheck.py (Diff revision 22) Col: 13 E128 continuation line under-indented for visual indent

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
bot/reviewbot/tools/shellcheck.py (Diff revision 23) Col: 19 E126 continuation line over-indented for hanging indent
-
bot/reviewbot/tools/shellcheck.py (Diff revision 23) Col: 19 E123 closing bracket does not match indentation of opening bracket's line
-
bot/reviewbot/tools/shellcheck.py (Diff revision 23) Col: 17 E123 closing bracket does not match indentation of opening bracket's line
-
bot/reviewbot/tools/shellcheck.py (Diff revision 23) Col: 17 E123 closing bracket does not match indentation of opening bracket's line
-
bot/reviewbot/tools/shellcheck.py (Diff revision 23) Col: 13 E123 closing bracket does not match indentation of opening bracket's line

-
Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
bot/reviewbot/tools/shellcheck.py (Diff revision 24) Col: 19 E126 continuation line over-indented for hanging indent
-
bot/reviewbot/tools/shellcheck.py (Diff revision 24) Col: 19 E123 closing bracket does not match indentation of opening bracket's line
-
bot/reviewbot/tools/shellcheck.py (Diff revision 24) Col: 32 E126 continuation line over-indented for hanging indent
-
bot/reviewbot/tools/shellcheck.py (Diff revision 24) Col: 32 E126 continuation line over-indented for hanging indent
-
bot/reviewbot/tools/shellcheck.py (Diff revision 24) Col: 24 E126 continuation line over-indented for hanging indent

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
bot/reviewbot/tools/shellcheck.py (Diff revision 25) Col: 19 E128 continuation line under-indented for visual indent

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
bot/reviewbot/tools/shellcheck.py (Diff revision 26) Col: 19 E126 continuation line over-indented for hanging indent
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 26) This doesn't belong in the docstring of the tool.
-
bot/reviewbot/tools/shellcheck.py (Diff revision 26) These lines should be indented 4 relative to
description
. -
bot/reviewbot/tools/shellcheck.py (Diff revision 26) This won't work as an
IntegerField
. This will have to be aCharField
or similar. -
bot/reviewbot/tools/shellcheck.py (Diff revision 26) Should only be one space after "Diagnostic".
Also, what if we want to ignore more than one error? This should accept a comma-separated list of codes.
-
bot/reviewbot/tools/shellcheck.py (Diff revision 26) This would be a good place to indicate the format of shellcheck output.
-
bot/reviewbot/tools/shellcheck.py (Diff revision 26) Re: above comments, this should check for a comma-separated list in this setting and loop through them.
Shellcheck may allow
--exclude=1,2,3,...
or you may have to use--exclude=1 --exclude=2 --exclude=3 ...
. -
bot/reviewbot/tools/shellcheck.py (Diff revision 26) Format this as:
output = execute(command_line, split_lines=True, ignore_errors=True)
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 26) This error message is useless. Also, you don't want to
print
(unless thats what other tools do for error logging).

-
Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
bot/reviewbot/tools/shellcheck.py (Diff revision 27) Col: 19 E126 continuation line over-indented for hanging indent
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revisions 26 - 27) Are we absolutely certain that we don't want to get JSON output from the shellcheck tool, parse it, and iterate over the issues it returns instead?
I think that'd allow us to avoid the split, and the casting to int on the line number.

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
bot/reviewbot/tools/shellcheck.py (Diff revision 28) Col: 19 E126 continuation line over-indented for hanging indent

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
bot/reviewbot/tools/shellcheck.py (Diff revision 30) Col: 80 E501 line too long (80 > 79 characters)
-

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
bot/reviewbot/tools/shellcheck.py (Diff revision 31) Col: 80 E501 line too long (80 > 79 characters)
-

-
Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
-

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
What's the issue with testing and options?
-
bot/reviewbot/tools/shellcheck.py (Diff revision 33) Standard library imports should come first, then a blank line, then package-local imports.
-
bot/reviewbot/tools/shellcheck.py (Diff revision 33) There are way too many spaces in here. One space between words, one space between sentences.
-
bot/reviewbot/tools/shellcheck.py (Diff revision 33) This should probably be a comment, not a docstring.
-
bot/reviewbot/tools/shellcheck.py (Diff revision 33) Can we reformat this as such:
command_line.append('--format=%s' % self.settings['specify_dialect'])
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 33) We should include the
.split()
call inside thetry
block, and catchIndexError
-
Testing Done: |
|
||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 34 (+90) |

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 34) Col: 38 E127 continuation line over-indented for visual indent
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 34) Col: 38 E127 continuation line over-indented for visual indent
-

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 35) Col: 38 E127 continuation line over-indented for visual indent
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 35) Col: 38 E127 continuation line over-indented for visual indent
-

-
Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py

-
Tool: PEP8 Style Checker Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py Tool: Pyflakes Processed Files: bot/reviewbot/tools/shellcheck.py bot/setup.py
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 37) You should mention this accepts comma separated list.
-
Testing Done: |
|
---|
Testing Done: |
|
---|