Summary:

Write a ReviewBot plugin for ShellCheck

Review Request #7701 — Created Oct. 15, 2015 and discarded May 18, 2016, 2:35 p.m.

Information
Owner:	xutong
Repository:	ReviewBot
Branch:	release-0.2.x
Bugs:
Depends On:
Reviewers
Groups:	reviewbot
People:

Description

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.

Testing Done


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.

Issues

Description	From	Last Updated
Col: 1 E265 block comment should start with '# '	reviewbot	Oct. 15, 2015, 9:26 p.m.
Col: 1 E265 block comment should start with '# '	reviewbot	Oct. 15, 2015, 9:26 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Oct. 15, 2015, 9:57 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:27 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:27 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:27 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:27 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:28 p.m.
Col: 2 E265 block comment should start with '# '	reviewbot	Oct. 15, 2015, 9:28 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:28 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:28 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:28 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:28 p.m.
Col: 1 E101 indentation contains mixed spaces and tabs	reviewbot	Oct. 15, 2015, 9:28 p.m.
Col: 9 E112 expected an indented block	reviewbot	Oct. 15, 2015, 9:58 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Oct. 15, 2015, 9:29 p.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Oct. 15, 2015, 9:29 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Oct. 15, 2015, 9:29 p.m.
Col: 80 E501 line too long (85 > 79 characters)	reviewbot	Oct. 15, 2015, 9:29 p.m.
Col: 80 E501 line too long (91 > 79 characters)	reviewbot	Oct. 15, 2015, 9:29 p.m.
Col: 80 E501 line too long (95 > 79 characters)	reviewbot	Oct. 15, 2015, 9:29 p.m.
Col: 80 E501 line too long (89 > 79 characters)	reviewbot	Oct. 15, 2015, 9:29 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Oct. 15, 2015, 9:29 p.m.
Col: 80 E501 line too long (107 > 79 characters)	reviewbot	Oct. 15, 2015, 9:29 p.m.
Col: 25 W291 trailing whitespace	reviewbot	Oct. 15, 2015, 9:29 p.m.
Col: 57 W291 trailing whitespace	reviewbot	Oct. 15, 2015, 9:29 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Oct. 15, 2015, 9:29 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Oct. 15, 2015, 9:29 p.m.
Col: 20 W291 trailing whitespace	reviewbot	Oct. 15, 2015, 9:29 p.m.
Col: 1 E265 block comment should start with '# '	reviewbot	Oct. 15, 2015, 9:29 p.m.
Col: 1 E265 block comment should start with '# '	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 2 E265 block comment should start with '# '	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 1 E101 indentation contains mixed spaces and tabs	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 9 E112 expected an indented block	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 80 E501 line too long (85 > 79 characters)	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 80 E501 line too long (91 > 79 characters)	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 80 E501 line too long (95 > 79 characters)	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 80 E501 line too long (89 > 79 characters)	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 80 E501 line too long (107 > 79 characters)	reviewbot	Oct. 15, 2015, 9:30 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Oct. 15, 2015, 9:31 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:31 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:31 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:31 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:31 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:31 p.m.
Col: 2 E303 too many blank lines (2)	reviewbot	Oct. 15, 2015, 9:31 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:31 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:31 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:31 p.m.
Col: 1 E101 indentation contains mixed spaces and tabs	reviewbot	Oct. 15, 2015, 9:31 p.m.
Col: 9 E112 expected an indented block	reviewbot	Oct. 15, 2015, 9:31 p.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Oct. 15, 2015, 9:31 p.m.
Col: 20 W292 no newline at end of file	reviewbot	Oct. 15, 2015, 9:31 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Oct. 15, 2015, 9:59 p.m.
Col: 2 E303 too many blank lines (2)	reviewbot	Oct. 15, 2015, 9:59 p.m.
Col: 2 E113 unexpected indentation	reviewbot	Oct. 15, 2015, 9:59 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:59 p.m.
Col: 1 E101 indentation contains mixed spaces and tabs	reviewbot	Oct. 15, 2015, 9:59 p.m.
Col: 9 E112 expected an indented block	reviewbot	Oct. 15, 2015, 9:59 p.m.
Col: 1 E101 indentation contains mixed spaces and tabs	reviewbot	Oct. 15, 2015, 9:59 p.m.
Col: 1 E101 indentation contains mixed spaces and tabs	reviewbot	Oct. 15, 2015, 9:59 p.m.
Col: 1 W191 indentation contains tabs	reviewbot	Oct. 15, 2015, 9:59 p.m.
Col: 1 E101 indentation contains mixed spaces and tabs	reviewbot	Oct. 15, 2015, 9:59 p.m.
Col: 20 W292 no newline at end of file	reviewbot	Oct. 15, 2015, 9:59 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Oct. 15, 2015, 9:58 p.m.
Col: 5 E112 expected an indented block	reviewbot	Oct. 15, 2015, 9:58 p.m.
Needs a docstring.	chipx86	Oct. 21, 2015, 12:56 p.m.
Let's start the versioning at 0.1 instead of 0.4.1, since this is the first version of the ShellCheck tool.	anselina	Oct. 21, 2015, 12:58 p.m.
"script" Missing period.	chipx86	Oct. 21, 2015, 12:56 p.m.
Remove this comment since the code is self-explanatory.	anselina	Oct. 21, 2015, 12:58 p.m.
I thought we talked about getting the JSON output format so that we could parse it more easily and robustly …	mike_conley	Oct. 16, 2015, 10:55 a.m.
Make sure these gracefully handle unexpected output. Also, variable names must always be in "this_case" and not "ThisCase".	chipx86	Oct. 21, 2015, 1:10 p.m.
You can use python tuple unpacking to make this much nicer: line_num, col_num, msg = line.split(':', 3) As Christian said, …	david	Nov. 7, 2015, 12:14 p.m.
Col: 67 W291 trailing whitespace	reviewbot	Oct. 23, 2015, 6:23 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Oct. 23, 2015, 6:18 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Oct. 23, 2015, 6:18 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Oct. 23, 2015, 6:18 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Oct. 23, 2015, 6:18 p.m.
Col: 70 W291 trailing whitespace	reviewbot	Oct. 23, 2015, 6:19 p.m.
Col: 67 W291 trailing whitespace	reviewbot	Oct. 23, 2015, 6:19 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Oct. 23, 2015, 6:19 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Oct. 23, 2015, 6:19 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Oct. 23, 2015, 6:19 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Oct. 23, 2015, 6:19 p.m.
Col: 70 W291 trailing whitespace	reviewbot	Oct. 23, 2015, 6:19 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Oct. 23, 2015, 6:25 p.m.
Col: 61 W291 trailing whitespace	reviewbot	Oct. 23, 2015, 6:25 p.m.
Can you remove this, since it doesn't really add any valuable infomation for the class?	anselina	Oct. 30, 2015, 3:13 p.m.
s/Ruturn/Return	anselina	Oct. 30, 2015, 3:15 p.m.
Add a blank line before this line.	anselina	Oct. 30, 2015, 3:15 p.m.
Variables should use lowercase words separated by underscores (e.g. Line_Num would be line_num instead).	anselina	Oct. 30, 2015, 3:15 p.m.
'os' imported but unused	reviewbot	Nov. 10, 2015, 11:38 p.m.
'make_tempfile' imported but unused	reviewbot	Nov. 10, 2015, 11:38 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Nov. 10, 2015, 11:38 p.m.
This has been mentioned before, but variable names should be lower-case, separated by underscores (e.g. file_name instead of File_name).	david	Oct. 30, 2015, 3:16 p.m.
Col: 14 E111 indentation is not a multiple of four	reviewbot	Nov. 10, 2015, 11:38 p.m.
Col: 14 E111 indentation is not a multiple of four	reviewbot	Nov. 10, 2015, 11:38 p.m.
'os' imported but unused	reviewbot	Nov. 10, 2015, 11:40 p.m.
'make_tempfile' imported but unused	reviewbot	Nov. 10, 2015, 11:40 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Nov. 10, 2015, 11:40 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Nov. 10, 2015, 11:40 p.m.
Col: 14 E111 indentation is not a multiple of four	reviewbot	Nov. 10, 2015, 11:40 p.m.
Col: 14 E111 indentation is not a multiple of four	reviewbot	Nov. 10, 2015, 11:40 p.m.
Col: 54 E201 whitespace after '('	reviewbot	Nov. 10, 2015, 11:40 p.m.
Col: 17 E128 continuation line under-indented for visual indent	reviewbot	Nov. 10, 2015, 11:41 p.m.
'os' imported but unused	reviewbot	Nov. 10, 2015, 11:41 p.m.
'make_tempfile' imported but unused	reviewbot	Nov. 10, 2015, 11:39 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Nov. 10, 2015, 11:39 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Nov. 10, 2015, 11:39 p.m.
Col: 14 E111 indentation is not a multiple of four	reviewbot	Nov. 10, 2015, 11:39 p.m.
Col: 14 E111 indentation is not a multiple of four	reviewbot	Nov. 10, 2015, 11:39 p.m.
Col: 54 E201 whitespace after '('	reviewbot	Nov. 10, 2015, 11:39 p.m.
Col: 17 E128 continuation line under-indented for visual indent	reviewbot	Nov. 10, 2015, 11:39 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Nov. 10, 2015, 11:42 p.m.
Col: 14 E111 indentation is not a multiple of four	reviewbot	Nov. 10, 2015, 11:42 p.m.
Col: 14 E111 indentation is not a multiple of four	reviewbot	Nov. 10, 2015, 11:42 p.m.
Couple of things here: - The first line of this docstring should describe what the class is, instead of stating …	anselina	Nov. 2, 2015, 8:55 p.m.
This does not return anything. It is a class.	brennie	Nov. 2, 2015, 8:45 p.m.
Remove this line.	anselina	Nov. 2, 2015, 8:50 p.m.
Since ShellCheck also identifies potential failures in addition to just style errors, let's update the description to: Checks shell scripts …	anselina	Nov. 2, 2015, 8:49 p.m.
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	anselina	Nov. 4, 2015, 6:01 p.m.
Comments should be written in full sentences, like: # Ignore the file if it is not a shell script.	anselina	Nov. 2, 2015, 8:48 p.m.
I think the code is pretty self-explanatory, so let's remove these comments?	anselina	Nov. 2, 2015, 8:48 p.m.
It'd be useful to add a comment above this line showing an example of shellcheck's output.	anselina	Nov. 2, 2015, 8:48 p.m.
These lines are over-indented by 1 space. Can we also rename mesg to msg?	anselina	Nov. 2, 2015, 8:46 p.m.
Col: 14 E111 indentation is not a multiple of four	reviewbot	Nov. 10, 2015, 11:39 p.m.
There's no need to make col_num an int here, so we can remove that.	anselina	Nov. 2, 2015, 8:45 p.m.
Col: 14 E111 indentation is not a multiple of four	reviewbot	Nov. 10, 2015, 11:38 p.m.
Blank line between these.	brennie	Nov. 6, 2015, midnight
Col: 80 E501 line too long (94 > 79 characters)	reviewbot	Nov. 6, 2015, midnight
This string should end with a space.	brennie	Nov. 6, 2015, midnight
Col: 82 W291 trailing whitespace	reviewbot	Nov. 6, 2015, midnight
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 6, 2015, midnight
Likewise.	brennie	Nov. 5, 2015, 11:59 p.m.
Col: 83 W291 trailing whitespace	reviewbot	Nov. 6, 2015, midnight
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 6, 2015, midnight
But this one should not.	brennie	Nov. 5, 2015, 11:59 p.m.
Col: 80 E501 line too long (91 > 79 characters)	reviewbot	Nov. 6, 2015, midnight
This should be a ChoiceField, not a CharField	brennie	Nov. 5, 2015, 11:57 p.m.
Col: 80 E501 line too long (86 > 79 characters)	reviewbot	Nov. 6, 2015, midnight
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 6, 2015, midnight
Col: 34 W291 trailing whitespace	reviewbot	Nov. 6, 2015, midnight
Remove this comment.	brennie	Nov. 5, 2015, 11:57 p.m.
This would be good in the docstring of the function.	brennie	Nov. 18, 2015, 9:48 p.m.
This can all be on one line.	brennie	Nov. 5, 2015, 11:57 p.m.
Col: 14 E111 indentation is not a multiple of four	reviewbot	Nov. 6, 2015, midnight
Col: 14 E111 indentation is not a multiple of four	reviewbot	Nov. 6, 2015, midnight
int(x) can fail if x is invalid input. This should have a try..except around it.	brennie	Nov. 7, 2015, 12:14 p.m.
Col: 65 W291 trailing whitespace	reviewbot	Nov. 6, 2015, 12:15 a.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Nov. 6, 2015, 12:15 a.m.
Col: 19 E128 continuation line under-indented for visual indent	reviewbot	Nov. 6, 2015, 12:15 a.m.
Col: 35 W291 trailing whitespace	reviewbot	Nov. 6, 2015, 12:14 a.m.
Col: 32 E225 missing whitespace around operator	reviewbot	Nov. 6, 2015, 12:14 a.m.
Col: 33 E225 missing whitespace around operator	reviewbot	Nov. 6, 2015, 12:14 a.m.
Col: 14 E111 indentation is not a multiple of four	reviewbot	Nov. 6, 2015, 12:22 a.m.
Col: 14 E111 indentation is not a multiple of four	reviewbot	Nov. 6, 2015, 12:22 a.m.
Col: 19 E128 continuation line under-indented for visual indent	reviewbot	Nov. 6, 2015, 12:23 a.m.
Col: 14 E111 indentation is not a multiple of four	reviewbot	Nov. 6, 2015, 12:23 a.m.
Col: 14 E111 indentation is not a multiple of four	reviewbot	Nov. 6, 2015, 12:23 a.m.
Col: 19 E128 continuation line under-indented for visual indent	reviewbot	Nov. 7, 2015, 12:17 p.m.
Col: 44 W291 trailing whitespace	reviewbot	Nov. 7, 2015, 12:17 p.m.
Col: 46 W291 trailing whitespace	reviewbot	Nov. 10, 2015, 11:43 p.m.
Col: 17 E128 continuation line under-indented for visual indent	reviewbot	Nov. 10, 2015, 11:43 p.m.
Col: 49 W291 trailing whitespace	reviewbot	Nov. 10, 2015, 11:43 p.m.
Col: 17 E128 continuation line under-indented for visual indent	reviewbot	Nov. 7, 2015, 12:17 p.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	Nov. 7, 2015, 12:18 p.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	Nov. 7, 2015, 12:18 p.m.
Col: 19 E128 continuation line under-indented for visual indent	reviewbot	Nov. 7, 2015, 12:20 p.m.
Col: 19 E124 closing bracket does not match visual indentation	reviewbot	Nov. 7, 2015, 12:20 p.m.
Col: 17 E128 continuation line under-indented for visual indent	reviewbot	Nov. 7, 2015, 12:20 p.m.
Col: 17 E124 closing bracket does not match visual indentation	reviewbot	Nov. 7, 2015, 12:20 p.m.
Col: 17 E128 continuation line under-indented for visual indent	reviewbot	Nov. 7, 2015, 12:20 p.m.
Col: 17 E124 closing bracket does not match visual indentation	reviewbot	Nov. 7, 2015, 12:20 p.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	Nov. 7, 2015, 12:21 p.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	Nov. 7, 2015, 12:21 p.m.
Col: 13 E124 closing bracket does not match visual indentation	reviewbot	Nov. 7, 2015, 12:21 p.m.
Col: 19 E128 continuation line under-indented for visual indent	reviewbot	Nov. 7, 2015, 2:21 p.m.
Col: 19 E124 closing bracket does not match visual indentation	reviewbot	Nov. 7, 2015, 2:21 p.m.
Col: 17 E128 continuation line under-indented for visual indent	reviewbot	Nov. 7, 2015, 2:21 p.m.
Col: 17 E124 closing bracket does not match visual indentation	reviewbot	Nov. 7, 2015, 2:21 p.m.
Col: 17 E128 continuation line under-indented for visual indent	reviewbot	Nov. 7, 2015, 2:21 p.m.
Col: 17 E124 closing bracket does not match visual indentation	reviewbot	Nov. 7, 2015, 2:20 p.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	Nov. 7, 2015, 2:20 p.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	Nov. 7, 2015, 2:20 p.m.
Col: 19 E126 continuation line over-indented for hanging indent	reviewbot	Nov. 10, 2015, 11:43 p.m.
Col: 19 E123 closing bracket does not match indentation of opening bracket's line	reviewbot	Nov. 7, 2015, 2:23 p.m.
Col: 17 E123 closing bracket does not match indentation of opening bracket's line	reviewbot	Nov. 7, 2015, 2:23 p.m.
Col: 17 E123 closing bracket does not match indentation of opening bracket's line	reviewbot	Nov. 7, 2015, 2:23 p.m.
Col: 13 E123 closing bracket does not match indentation of opening bracket's line	reviewbot	Nov. 7, 2015, 2:23 p.m.
Col: 19 E126 continuation line over-indented for hanging indent	reviewbot	Nov. 7, 2015, 2:26 p.m.
Col: 19 E123 closing bracket does not match indentation of opening bracket's line	reviewbot	Nov. 7, 2015, 2:26 p.m.
Col: 32 E126 continuation line over-indented for hanging indent	reviewbot	Nov. 7, 2015, 2:26 p.m.
Col: 32 E126 continuation line over-indented for hanging indent	reviewbot	Nov. 7, 2015, 2:26 p.m.
Col: 24 E126 continuation line over-indented for hanging indent	reviewbot	Nov. 7, 2015, 2:26 p.m.
Col: 19 E128 continuation line under-indented for visual indent	reviewbot	Nov. 7, 2015, 2:28 p.m.
This doesn't belong in the docstring of the tool.	brennie	Nov. 10, 2015, 11:43 p.m.
Col: 19 E126 continuation line over-indented for hanging indent	reviewbot	Nov. 10, 2015, 11:44 p.m.
These lines should be indented 4 relative to description.	brennie	Nov. 18, 2015, 9:46 p.m.
This won't work as an IntegerField. This will have to be a CharField or similar.	brennie	Nov. 18, 2015, 9:18 p.m.
Should only be one space after "Diagnostic". Also, what if we want to ignore more than one error? This should …	brennie	Nov. 10, 2015, 11:32 p.m.
This would be a good place to indicate the format of shellcheck output.	brennie	Nov. 10, 2015, 11:33 p.m.
Are we absolutely certain that we don't want to get JSON output from the shellcheck tool, parse it, and iterate …	mike_conley	Nov. 19, 2015, 10:47 p.m.
Re: above comments, this should check for a comma-separated list in this setting and loop through them. Shellcheck may allow …	brennie	Nov. 18, 2015, 9:45 p.m.
Format this as: output = execute(command_line, split_lines=True, ignore_errors=True)	brennie	Nov. 10, 2015, 11:35 p.m.
Blank line between these.	brennie	Nov. 10, 2015, 11:35 p.m.
This error message is useless. Also, you don't want to print (unless thats what other tools do for error logging).	brennie	Nov. 12, 2015, 11:07 a.m.
Col: 19 E126 continuation line over-indented for hanging indent	reviewbot	Nov. 12, 2015, 11:38 a.m.
Col: 26 E201 whitespace after '('	reviewbot	Nov. 12, 2015, 11:38 a.m.
Col: 19 E126 continuation line over-indented for hanging indent	reviewbot	Nov. 18, 2015, 11:44 a.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Nov. 18, 2015, 9:50 p.m.
Col: 52 W291 trailing whitespace	reviewbot	Nov. 18, 2015, 9:50 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Nov. 18, 2015, 9:52 p.m.
Col: 52 W291 trailing whitespace	reviewbot	Nov. 18, 2015, 9:52 p.m.
Col: 30 W291 trailing whitespace	reviewbot	Nov. 18, 2015, 9:53 p.m.
Col: 34 W291 trailing whitespace	reviewbot	Nov. 18, 2015, 9:53 p.m.
Standard library imports should come first, then a blank line, then package-local imports.	david	Nov. 24, 2015, 10:47 p.m.
There are way too many spaces in here. One space between words, one space between sentences.	david	Nov. 24, 2015, 9:52 p.m.
This should probably be a comment, not a docstring.	david	Nov. 24, 2015, 9:52 p.m.
Can we reformat this as such: command_line.append('--format=%s' % self.settings['specify_dialect'])	david	Nov. 24, 2015, 9:53 p.m.
Same here.	david	Nov. 24, 2015, 9:54 p.m.
We should include the .split() call inside the try block, and catch IndexError	david	Nov. 24, 2015, 9:55 p.m.
This is a very poor error message.	david	Nov. 25, 2015, 12:35 p.m.
Col: 50 W291 trailing whitespace	reviewbot	Nov. 24, 2015, 10:55 p.m.
Col: 38 E127 continuation line over-indented for visual indent	reviewbot	Nov. 24, 2015, 10:55 p.m.
Col: 51 W291 trailing whitespace	reviewbot	Nov. 24, 2015, 10:55 p.m.
Col: 38 E127 continuation line over-indented for visual indent	reviewbot	Nov. 24, 2015, 10:55 p.m.
Col: 52 W292 no newline at end of file	reviewbot	Nov. 24, 2015, 10:56 p.m.
Col: 50 W291 trailing whitespace	reviewbot	Nov. 24, 2015, 10:56 p.m.
Col: 38 E127 continuation line over-indented for visual indent	reviewbot	Nov. 24, 2015, 10:56 p.m.
Col: 51 W291 trailing whitespace	reviewbot	Nov. 24, 2015, 10:56 p.m.
Col: 38 E127 continuation line over-indented for visual indent	reviewbot	Nov. 24, 2015, 10:56 p.m.
Col: 52 W292 no newline at end of file	reviewbot	Nov. 24, 2015, 10:56 p.m.
You should mention this accepts comma separated list.	brennie	Nov. 28, 2015, 4:25 p.m.
This is not useful. Remove the try: block.	brennie	Nov. 28, 2015, 4:27 p.m.
Col: 20 W292 no newline at end of file	reviewbot	Dec. 5, 2015, 12:33 a.m.

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

bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 1
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 1
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 2
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```

bot/reviewbot/tools/shellcheck.py (Diff revision 1)

Show all issues

Col: 1
 E101 indentation contains mixed spaces and tabs

bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 9
 E112 expected an indented block
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 9
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 80
 E501 line too long (85 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 80
 E501 line too long (91 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 80
 E501 line too long (95 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 80
 E501 line too long (89 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 80
 E501 line too long (107 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 25
 W291 trailing whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 57
 W291 trailing whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 1)
Show all issues
```
Col: 20
 W291 trailing whitespace
```

Diff:

Revision 2 (+60)

Show changes

bot/reviewbot/tools/shellcheck.py

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

bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 1
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 1
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 2
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```

bot/reviewbot/tools/shellcheck.py (Diff revision 2)

Show all issues

Col: 1
 E101 indentation contains mixed spaces and tabs

bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 9
 E112 expected an indented block
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 9
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (85 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (91 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (95 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (89 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (107 > 79 characters)
```

Diff:

Revision 3 (+49)

Show changes

bot/reviewbot/tools/shellcheck.py

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

bot/reviewbot/tools/shellcheck.py (Diff revision 3)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
bot/reviewbot/tools/shellcheck.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 3)
Show all issues
```
Col: 2
 E303 too many blank lines (2)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```
bot/reviewbot/tools/shellcheck.py (Diff revision 3)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```

bot/reviewbot/tools/shellcheck.py (Diff revision 3)

Show all issues

Col: 1
 E101 indentation contains mixed spaces and tabs

bot/reviewbot/tools/shellcheck.py (Diff revision 3)
Show all issues
```
Col: 9
 E112 expected an indented block
```
bot/reviewbot/tools/shellcheck.py (Diff revision 3)
Show all issues
```
Col: 9
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 3)
Show all issues
```
Col: 20
 W292 no newline at end of file
```

Diff:

Revision 4 (+49)

Show changes

bot/reviewbot/tools/shellcheck.py

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

bot/reviewbot/tools/shellcheck.py (Diff revision 4)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
bot/reviewbot/tools/shellcheck.py (Diff revision 4)
Show all issues
```
Col: 2
 E303 too many blank lines (2)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 4)
Show all issues
```
Col: 2
 E113 unexpected indentation
```
bot/reviewbot/tools/shellcheck.py (Diff revision 4)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```

bot/reviewbot/tools/shellcheck.py (Diff revision 4)

Show all issues

Col: 1
 E101 indentation contains mixed spaces and tabs

bot/reviewbot/tools/shellcheck.py (Diff revision 4)
Show all issues
```
Col: 9
 E112 expected an indented block
```

bot/reviewbot/tools/shellcheck.py (Diff revision 4)

Show all issues

Col: 1
 E101 indentation contains mixed spaces and tabs

bot/reviewbot/tools/shellcheck.py (Diff revision 4)

Show all issues

Col: 1
 E101 indentation contains mixed spaces and tabs

bot/reviewbot/tools/shellcheck.py (Diff revision 4)
Show all issues
```
Col: 1
 W191 indentation contains tabs
```

bot/reviewbot/tools/shellcheck.py (Diff revision 4)

Show all issues

Col: 1
 E101 indentation contains mixed spaces and tabs

bot/reviewbot/tools/shellcheck.py (Diff revision 4)
Show all issues
```
Col: 20
 W292 no newline at end of file
```

Diff:

Revision 5 (+49)

Show changes

bot/reviewbot/tools/shellcheck.py

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

bot/reviewbot/tools/shellcheck.py (Diff revision 5)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
bot/reviewbot/tools/shellcheck.py (Diff revision 5)
Show all issues
```
Col: 5
 E112 expected an indented block
```

Diff:

Revision 6 (+50)

Show changes

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)

Show all issues

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?

xutong Oct. 16, 2015, 10:55 a.m.

This is a work in process code. Next step, I want to test it and see what the output looks like, then optimize it.

david Oct. 16, 2015, 1:33 p.m.
```
Can you add "WIP" to the summary line?
```

Summary:

-	write a new shellcheck tool plugin
+	write a new shellcheck tool plugin[WIP]

bot/reviewbot/tools/shellcheck.py (Diff revision 6)
Show all issues
```
Needs a docstring.
```
bot/reviewbot/tools/shellcheck.py (Diff revision 6)
Show all issues
```
"script"
Missing period.
```

bot/reviewbot/tools/shellcheck.py (Diff revision 6)

Show all issues

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)

Show all issues

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.

xutong Nov. 6, 2015, 12:22 a.m.

Hi, David, which error should I use try/except. I got a little confused here, becasue all the input should be
right, if the excute() is correct.

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)

Show all issues

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)
Show all issues
```
Remove this comment since the code is self-explanatory.
```

Diff:

Revision 7 (+54)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 7)
Show all issues
```
Col: 67
 W291 trailing whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 7)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 7)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 7)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 7)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
bot/setup.py (Diff revision 7)
Show all issues
```
Col: 70
 W291 trailing whitespace
```

Diff:

Revision 8 (+53)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 8)
Show all issues
```
Col: 67
 W291 trailing whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 8)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 8)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 8)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 8)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
bot/setup.py (Diff revision 8)
Show all issues
```
Col: 70
 W291 trailing whitespace
```

Diff:

Revision 9 (+56)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 9)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 9)
Show all issues
```
Col: 61
 W291 trailing whitespace
```

Diff:

Revision 10 (+56)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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)

Show all issues

Can you remove this, since it doesn't really add any valuable infomation for the class?

bot/reviewbot/tools/shellcheck.py (Diff revision 10)
Show all issues
```
s/Ruturn/Return
```
bot/reviewbot/tools/shellcheck.py (Diff revision 10)
Show all issues
```
Add a blank line before this line.
```

bot/reviewbot/tools/shellcheck.py (Diff revision 10)

Show all issues

Variables should use lowercase words separated by underscores (e.g. Line_Num would be line_num instead).

Branch:

-	master
+	release-0.2.x

Diff:

Revision 11 (+55)

Show changes

bot/reviewbot/tools/shellcheck.py

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



Tool: Pyflakes
Processed Files:
    bot/reviewbot/tools/shellcheck.py

Diff:

Revision 12 (+242)

Show changes

	bot/setup.py
	bot/reviewbot/tools/buildbot.py
	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)
Show all issues
```
 'os' imported but unused
```
bot/reviewbot/tools/buildbot.py (Diff revision 12)
Show all issues
```
 'make_tempfile' imported but unused
```
bot/reviewbot/tools/buildbot.py (Diff revision 12)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 12)
Show all issues
```
Col: 14
 E111 indentation is not a multiple of four
```
bot/reviewbot/tools/shellcheck.py (Diff revision 12)
Show all issues
```
Col: 14
 E111 indentation is not a multiple of four
```

bot/reviewbot/tools/shellcheck.py (Diff revision 12)

Show all issues

This has been mentioned before, but variable names should be lower-case, separated by underscores (e.g. file_name instead of File_name).

xutong Oct. 29, 2015, 11:21 a.m.

Sorry, I will work on that latter. I just post the code to save what I did right now.
I already done the test of shellcheck plugin. I will fix the error. Thanks.

Diff:

Revision 13 (+239)

Show changes

	bot/setup.py
	bot/reviewbot/tools/buildbot.py
	bot/reviewbot/tools/shellcheck.py

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)
Show all issues
```
 'os' imported but unused
```
bot/reviewbot/tools/buildbot.py (Diff revision 13)
Show all issues
```
 'make_tempfile' imported but unused
```
bot/reviewbot/tools/buildbot.py (Diff revision 13)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 13)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 13)
Show all issues
```
Col: 14
 E111 indentation is not a multiple of four
```
bot/reviewbot/tools/shellcheck.py (Diff revision 13)
Show all issues
```
Col: 14
 E111 indentation is not a multiple of four
```
bot/reviewbot/tools/shellcheck.py (Diff revision 13)
Show all issues
```
Col: 54
 E201 whitespace after '('
```

bot/reviewbot/tools/shellcheck.py (Diff revision 13)

Show all issues

Col: 17
 E128 continuation line under-indented for visual indent

Diff:

Revision 14 (+239)

Show changes

	bot/setup.py
	bot/reviewbot/tools/buildbot.py
	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 14)
Show all issues
```
 'os' imported but unused
```
bot/reviewbot/tools/buildbot.py (Diff revision 14)
Show all issues
```
 'make_tempfile' imported but unused
```
bot/reviewbot/tools/buildbot.py (Diff revision 14)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
bot/reviewbot/tools/shellcheck.py (Diff revision 14)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 14)
Show all issues
```
Col: 14
 E111 indentation is not a multiple of four
```
bot/reviewbot/tools/shellcheck.py (Diff revision 14)
Show all issues
```
Col: 14
 E111 indentation is not a multiple of four
```
bot/reviewbot/tools/shellcheck.py (Diff revision 14)
Show all issues
```
Col: 54
 E201 whitespace after '('
```

bot/reviewbot/tools/shellcheck.py (Diff revision 14)

Show all issues

Col: 17
 E128 continuation line under-indented for visual indent

Diff:

Revision 15 (+52)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 15)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 15)
Show all issues
```
Col: 14
 E111 indentation is not a multiple of four
```
bot/reviewbot/tools/shellcheck.py (Diff revision 15)
Show all issues
```
Col: 14
 E111 indentation is not a multiple of four
```

Summary:

-	write a new shellcheck tool plugin[WIP]
+	Write a ReviewBot plugin for ShellCheck[WIP]

Description:

~		write a new shellcheck tool plugin[WIP]
	~	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.

Testing Done:

	+	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.
	+	Run unit tests on reviewboard, see if the new plugin break any other features of reviewboard.

Diff:

Revision 16 (+52)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 16)
Show all issues
```
Col: 14
 E111 indentation is not a multiple of four
```
bot/reviewbot/tools/shellcheck.py (Diff revision 16)
Show all issues
```
Col: 14
 E111 indentation is not a multiple of four
```

Summary:

-	Write a ReviewBot plugin for ShellCheck[WIP]
+	Write a ReviewBot plugin for ShellCheck

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)

Show all issues

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 the handle_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)
Show all issues
```
Remove this line.
```

bot/reviewbot/tools/shellcheck.py (Diff revision 16)

Show all issues

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)

Show all issues

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)

Show all issues

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)

Show all issues

I think the code is pretty self-explanatory, so let's remove these comments?

bot/reviewbot/tools/shellcheck.py (Diff revision 16)

Show all issues

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)

Show all issues

These lines are over-indented by 1 space. Can we also rename mesg to msg?

bot/reviewbot/tools/shellcheck.py (Diff revision 16)

Show all issues

There's no need to make col_num an int here, so we can remove that.

bot/reviewbot/tools/shellcheck.py (Diff revision 16)
Show all issues
```
This does not return anything. It is a class.
```

Testing Done:

~		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.
~		Run unit tests on reviewboard, see if the new plugin break any other features of reviewboard.
	~	can not been test right now after add opnions. The local dev server does not work.
	~	[1. 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.
	+
	+	Run unit tests on reviewboard, see if the new plugin break any other features of reviewboard.]

Diff:

Revision 17 (+67)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 17)
Show all issues
```
Col: 80
 E501 line too long (94 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
Col: 82
 W291 trailing whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
Col: 83
 W291 trailing whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
Col: 80
 E501 line too long (91 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
Col: 80
 E501 line too long (86 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
Col: 34
 W291 trailing whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
Col: 14
 E111 indentation is not a multiple of four
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
Col: 14
 E111 indentation is not a multiple of four
```

bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
Blank line between these.
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
This string should end with a space.
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
Likewise.
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
But this one should not.
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
This should be a ChoiceField, not a CharField
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
Remove this comment.
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
This would be good in the docstring of the function.
```
bot/reviewbot/tools/shellcheck.py (Diff revision 17)
Show all issues
```
This can all be on one line.
```

bot/reviewbot/tools/shellcheck.py (Diff revision 17)

Show all issues

int(x) can fail if x is invalid input. This should have a try..except around it.

xutong Nov. 6, 2015, 12:20 a.m.

Hi, Barret, line_num is already a string which represent the line number. There will not be some invalid input, so, why should I use the try..expect?

Diff:

Revision 18 (+78)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 18)
Show all issues
```
Col: 65
 W291 trailing whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 18)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```

bot/reviewbot/tools/shellcheck.py (Diff revision 18)

Show all issues

Col: 19
 E128 continuation line under-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 18)
Show all issues
```
Col: 35
 W291 trailing whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 18)
Show all issues
```
Col: 32
 E225 missing whitespace around operator
```
bot/reviewbot/tools/shellcheck.py (Diff revision 18)
Show all issues
```
Col: 33
 E225 missing whitespace around operator
```
bot/reviewbot/tools/shellcheck.py (Diff revision 18)
Show all issues
```
Col: 14
 E111 indentation is not a multiple of four
```
bot/reviewbot/tools/shellcheck.py (Diff revision 18)
Show all issues
```
Col: 14
 E111 indentation is not a multiple of four
```

Diff:

Revision 19 (+78)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 19)

Show all issues

Col: 19
 E128 continuation line under-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 19)
Show all issues
```
Col: 14
 E111 indentation is not a multiple of four
```
bot/reviewbot/tools/shellcheck.py (Diff revision 19)
Show all issues
```
Col: 14
 E111 indentation is not a multiple of four
```

Diff:

Revision 20 (+86)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 20)

Show all issues

Col: 19
 E128 continuation line under-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 20)
Show all issues
```
Col: 44
 W291 trailing whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 20)
Show all issues
```
Col: 46
 W291 trailing whitespace
```

bot/reviewbot/tools/shellcheck.py (Diff revision 20)

Show all issues

Col: 17
 E128 continuation line under-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 20)
Show all issues
```
Col: 49
 W291 trailing whitespace
```

bot/reviewbot/tools/shellcheck.py (Diff revision 20)

Show all issues

Col: 17
 E128 continuation line under-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 20)

Show all issues

Col: 13
 E128 continuation line under-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 20)

Show all issues

Col: 13
 E128 continuation line under-indented for visual indent

Diff:

Revision 21 (+90)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 21)

Show all issues

Col: 19
 E128 continuation line under-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 21)

Show all issues

Col: 19
 E124 closing bracket does not match visual indentation

bot/reviewbot/tools/shellcheck.py (Diff revision 21)

Show all issues

Col: 17
 E128 continuation line under-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 21)

Show all issues

Col: 17
 E124 closing bracket does not match visual indentation

bot/reviewbot/tools/shellcheck.py (Diff revision 21)

Show all issues

Col: 17
 E128 continuation line under-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 21)

Show all issues

Col: 17
 E124 closing bracket does not match visual indentation

bot/reviewbot/tools/shellcheck.py (Diff revision 21)

Show all issues

Col: 13
 E128 continuation line under-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 21)

Show all issues

Col: 13
 E128 continuation line under-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 21)

Show all issues

Col: 13
 E124 closing bracket does not match visual indentation

Testing Done:

		can not been test right now after add opnions. The local dev server does not work.
~		[1. Test shellcheck plugin:
	~	1. 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.

~		Run unit tests on reviewboard, see if the new plugin break any other features of reviewboard.]
	~	Run unit tests on reviewboard, see if the new plugin break any other features of reviewboard.

Diff:

Revision 22 (+89)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 22)

Show all issues

Col: 19
 E128 continuation line under-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 22)

Show all issues

Col: 19
 E124 closing bracket does not match visual indentation

bot/reviewbot/tools/shellcheck.py (Diff revision 22)

Show all issues

Col: 17
 E128 continuation line under-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 22)

Show all issues

Col: 17
 E124 closing bracket does not match visual indentation

bot/reviewbot/tools/shellcheck.py (Diff revision 22)

Show all issues

Col: 17
 E128 continuation line under-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 22)

Show all issues

Col: 17
 E124 closing bracket does not match visual indentation

bot/reviewbot/tools/shellcheck.py (Diff revision 22)

Show all issues

Col: 13
 E128 continuation line under-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 22)

Show all issues

Col: 13
 E128 continuation line under-indented for visual indent

Diff:

Revision 23 (+92)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 23)

Show all issues

Col: 19
 E126 continuation line over-indented for hanging indent

bot/reviewbot/tools/shellcheck.py (Diff revision 23)

Show all issues

Col: 19
 E123 closing bracket does not match indentation of opening bracket's line

bot/reviewbot/tools/shellcheck.py (Diff revision 23)

Show all issues

Col: 17
 E123 closing bracket does not match indentation of opening bracket's line

bot/reviewbot/tools/shellcheck.py (Diff revision 23)

Show all issues

Col: 17
 E123 closing bracket does not match indentation of opening bracket's line

bot/reviewbot/tools/shellcheck.py (Diff revision 23)

Show all issues

Col: 13
 E123 closing bracket does not match indentation of opening bracket's line

Diff:

Revision 24 (+92)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 24)

Show all issues

Col: 19
 E126 continuation line over-indented for hanging indent

bot/reviewbot/tools/shellcheck.py (Diff revision 24)

Show all issues

Col: 19
 E123 closing bracket does not match indentation of opening bracket's line

bot/reviewbot/tools/shellcheck.py (Diff revision 24)

Show all issues

Col: 32
 E126 continuation line over-indented for hanging indent

bot/reviewbot/tools/shellcheck.py (Diff revision 24)

Show all issues

Col: 32
 E126 continuation line over-indented for hanging indent

bot/reviewbot/tools/shellcheck.py (Diff revision 24)

Show all issues

Col: 24
 E126 continuation line over-indented for hanging indent

Diff:

Revision 25 (+87)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 25)

Show all issues

Col: 19
 E128 continuation line under-indented for visual indent

Diff:

Revision 26 (+88)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 26)

Show all issues

Col: 19
 E126 continuation line over-indented for hanging indent

bot/reviewbot/tools/shellcheck.py (Diff revision 26)
Show all issues
```
This doesn't belong in the docstring of the tool.
```

bot/reviewbot/tools/shellcheck.py (Diff revision 26)

Show all issues

These lines should be indented 4 relative to description.

xutong Nov. 10, 2015, 11:36 p.m.

What do you mean indented 4 relative. What the 4 relative stand for?

brennie Nov. 18, 2015, 7:25 p.m.


    description = (
        'Checks shell scripts for ...'
        'potential ...')

bot/reviewbot/tools/shellcheck.py (Diff revision 26)

Show all issues

This won't work as an IntegerField. This will have to be a CharField or similar.

xutong Nov. 12, 2015, 11:46 a.m.

Once I changed IntegerField to CharField, it shows that:
reviewbot.tasks.ProcessReviewRequest[6f95c052-b3c7-4599-b94e-08d13401a596]:
Error executing tool: need more than 2 values to unpack
I do not know how can I fix it..

bot/reviewbot/tools/shellcheck.py (Diff revision 26)

Show all issues

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)

Show all issues

This would be a good place to indicate the format of shellcheck output.

bot/reviewbot/tools/shellcheck.py (Diff revision 26)

Show all issues

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 ....

anselina Nov. 11, 2015, 7:45 a.m.

Barret is right - you should make this option a CharField and let multiple codes get ignored with a comma-separated list. See https://github.com/reviewboard/ReviewBot/blob/master/bot/reviewbot/tools/pep8.py#L22 for an example. It looks like Shellcheck does allow --exclude=1,2,3.

bot/reviewbot/tools/shellcheck.py (Diff revision 26)

Show all issues

Format this as:

output = execute(command_line, split_lines=True, ignore_errors=True)

bot/reviewbot/tools/shellcheck.py (Diff revision 26)
Show all issues
```
Blank line between these.
```

bot/reviewbot/tools/shellcheck.py (Diff revision 26)

Show all issues

This error message is useless. Also, you don't want to print (unless thats what other tools do for error logging).

anselina Nov. 11, 2015, 7:45 a.m.

Yes, use logging.error() instead of print().

Diff:

Revision 27 (+85)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 27)

Show all issues

Col: 19
 E126 continuation line over-indented for hanging indent

bot/reviewbot/tools/shellcheck.py (Diff revision 27)
Show all issues
```
Col: 26
 E201 whitespace after '('
```

bot/reviewbot/tools/shellcheck.py (Diff revisions 26 - 27)

Show all issues

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.

xutong Nov. 12, 2015, 11:45 a.m.

Hi, Mike
I used line.split(':', 3)[1:] to splite line, and
use try/catch to handle the error. I will think about it.
I think I will talk about it and optimize codes, after I fix other requests.
Thanks,
Xutong Liu

xutong Nov. 18, 2015, 9:46 p.m.

I will set fixed it by now. And tommorow I will ask Anselina Chia about it.

Diff:

Revision 28 (+86)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 28)

Show all issues

Col: 19
 E126 continuation line over-indented for hanging indent

Diff:

Revision 29 (+84)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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

Diff:

Revision 30 (+90)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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)
Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 30)
Show all issues
```
Col: 52
 W291 trailing whitespace
```

Diff:

Revision 31 (+90)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 31)
Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
bot/reviewbot/tools/shellcheck.py (Diff revision 31)
Show all issues
```
Col: 52
 W291 trailing whitespace
```

Diff:

Revision 32 (+93)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 32)
Show all issues
```
Col: 30
 W291 trailing whitespace
```
bot/reviewbot/tools/shellcheck.py (Diff revision 32)
Show all issues
```
Col: 34
 W291 trailing whitespace
```

Diff:

Revision 33 (+93)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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)

Show all issues

Standard library imports should come first, then a blank line, then package-local imports.

bot/reviewbot/tools/shellcheck.py (Diff revision 33)

Show all issues

There are way too many spaces in here. One space between words, one space between sentences.

bot/reviewbot/tools/shellcheck.py (Diff revision 33)

Show all issues

This should probably be a comment, not a docstring.

bot/reviewbot/tools/shellcheck.py (Diff revision 33)

Show all issues

Can we reformat this as such:

command_line.append('--format=%s'
                    % self.settings['specify_dialect'])

bot/reviewbot/tools/shellcheck.py (Diff revision 33)
Show all issues
```
Same here.
```

bot/reviewbot/tools/shellcheck.py (Diff revision 33)

Show all issues

We should include the .split() call inside the try block, and catch IndexError

bot/reviewbot/tools/shellcheck.py (Diff revision 33)
Show all issues
```
This is a very poor error message.
```

Testing Done:

~		can not been test right now after add opnions. The local dev server does not work.
~		1. Test shellcheck plugin:
	~	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.
	~	Run unit tests on reviewboard, see if the new plugin break any other features of reviewboard.
-		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.
-
-		Run unit tests on reviewboard, see if the new plugin break any other features of reviewboard.

Diff:

Revision 34 (+90)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 34)
Show all issues
```
Col: 50
 W291 trailing whitespace
```

bot/reviewbot/tools/shellcheck.py (Diff revision 34)

Show all issues

Col: 38
 E127 continuation line over-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 34)
Show all issues
```
Col: 51
 W291 trailing whitespace
```

bot/reviewbot/tools/shellcheck.py (Diff revision 34)

Show all issues

Col: 38
 E127 continuation line over-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 34)
Show all issues
```
Col: 52
 W292 no newline at end of file
```

Diff:

Revision 35 (+91)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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 35)
Show all issues
```
Col: 50
 W291 trailing whitespace
```

bot/reviewbot/tools/shellcheck.py (Diff revision 35)

Show all issues

Col: 38
 E127 continuation line over-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 35)
Show all issues
```
Col: 51
 W291 trailing whitespace
```

bot/reviewbot/tools/shellcheck.py (Diff revision 35)

Show all issues

Col: 38
 E127 continuation line over-indented for visual indent

bot/reviewbot/tools/shellcheck.py (Diff revision 35)
Show all issues
```
Col: 52
 W292 no newline at end of file
```

Diff:

Revision 36 (+91)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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

Diff:

Revision 37 (+91)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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)
Show all issues
```
You should mention this accepts comma separated list.
```
bot/reviewbot/tools/shellcheck.py (Diff revision 37)
Show all issues
```
This is not useful. Remove the try: block.
```

Diff:

Revision 38 (+89)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.py

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

Testing Done:

~		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.
~		Run unit tests on reviewboard, see if the new plugin break any other features of reviewboard.
	~	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 orginal 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.

Testing Done:

~		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 orginal shellcheck in terminal, and see if all the output are correct.
	~	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.

Diff:

Revision 39 (+89)

Show changes

	bot/setup.py
	bot/reviewbot/tools/shellcheck.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