Write a ReviewBot plugin for ShellCheck

Review Request #7701 — Created Oct. 15, 2015 and discarded

xutong
ReviewBot
release-0.2.x
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.

  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.
    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.
  2. 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 '# '

reviewbotreviewbot

Col: 1 E265 block comment should start with '# '

reviewbotreviewbot

Col: 1 E302 expected 2 blank lines, found 1

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 2 E265 block comment should start with '# '

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 9 E112 expected an indented block

reviewbotreviewbot

Col: 1 W293 blank line contains whitespace

reviewbotreviewbot

Col: 9 E265 block comment should start with '# '

reviewbotreviewbot

Col: 1 W293 blank line contains whitespace

reviewbotreviewbot

Col: 80 E501 line too long (85 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (91 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (95 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (89 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (82 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (107 > 79 characters)

reviewbotreviewbot

Col: 25 W291 trailing whitespace

reviewbotreviewbot

Col: 57 W291 trailing whitespace

reviewbotreviewbot

Col: 1 W293 blank line contains whitespace

reviewbotreviewbot

Col: 1 W293 blank line contains whitespace

reviewbotreviewbot

Col: 20 W291 trailing whitespace

reviewbotreviewbot

Col: 1 E265 block comment should start with '# '

reviewbotreviewbot

Col: 1 E265 block comment should start with '# '

reviewbotreviewbot

Col: 1 E302 expected 2 blank lines, found 1

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 2 E265 block comment should start with '# '

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 9 E112 expected an indented block

reviewbotreviewbot

Col: 9 E265 block comment should start with '# '

reviewbotreviewbot

Col: 80 E501 line too long (85 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (91 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (95 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (89 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (82 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (107 > 79 characters)

reviewbotreviewbot

Col: 1 E302 expected 2 blank lines, found 1

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 2 E303 too many blank lines (2)

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 9 E112 expected an indented block

reviewbotreviewbot

Col: 9 E265 block comment should start with '# '

reviewbotreviewbot

Col: 20 W292 no newline at end of file

reviewbotreviewbot

Col: 1 E302 expected 2 blank lines, found 1

reviewbotreviewbot

Col: 2 E303 too many blank lines (2)

reviewbotreviewbot

Col: 2 E113 unexpected indentation

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 9 E112 expected an indented block

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 20 W292 no newline at end of file

reviewbotreviewbot

Col: 1 E302 expected 2 blank lines, found 1

reviewbotreviewbot

Col: 5 E112 expected an indented block

reviewbotreviewbot

Needs a docstring.

chipx86chipx86

Let's start the versioning at 0.1 instead of 0.4.1, since this is the first version of the ShellCheck tool.

anselinaanselina

"script" Missing period.

chipx86chipx86

Remove this comment since the code is self-explanatory.

anselinaanselina

I thought we talked about getting the JSON output format so that we could parse it more easily and robustly ...

mike_conleymike_conley

Make sure these gracefully handle unexpected output. Also, variable names must always be in "this_case" and not "ThisCase".

chipx86chipx86

You can use python tuple unpacking to make this much nicer: line_num, col_num, msg = line.split(':', 3) As Christian said, ...

daviddavid

Col: 67 W291 trailing whitespace

reviewbotreviewbot

Col: 13 E265 block comment should start with '# '

reviewbotreviewbot

Col: 13 E265 block comment should start with '# '

reviewbotreviewbot

Col: 13 E265 block comment should start with '# '

reviewbotreviewbot

Col: 13 E265 block comment should start with '# '

reviewbotreviewbot

Col: 70 W291 trailing whitespace

reviewbotreviewbot

Col: 67 W291 trailing whitespace

reviewbotreviewbot

Col: 13 E265 block comment should start with '# '

reviewbotreviewbot

Col: 13 E265 block comment should start with '# '

reviewbotreviewbot

Col: 13 E265 block comment should start with '# '

reviewbotreviewbot

Col: 13 E265 block comment should start with '# '

reviewbotreviewbot

Col: 70 W291 trailing whitespace

reviewbotreviewbot

Col: 1 W293 blank line contains whitespace

reviewbotreviewbot

Col: 61 W291 trailing whitespace

reviewbotreviewbot

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

anselinaanselina

s/Ruturn/Return

anselinaanselina

Add a blank line before this line.

anselinaanselina

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

anselinaanselina

'os' imported but unused

reviewbotreviewbot

'make_tempfile' imported but unused

reviewbotreviewbot

Col: 13 E265 block comment should start with '# '

reviewbotreviewbot

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

daviddavid

Col: 14 E111 indentation is not a multiple of four

reviewbotreviewbot

Col: 14 E111 indentation is not a multiple of four

reviewbotreviewbot

'os' imported but unused

reviewbotreviewbot

'make_tempfile' imported but unused

reviewbotreviewbot

Col: 13 E265 block comment should start with '# '

reviewbotreviewbot

Col: 1 W293 blank line contains whitespace

reviewbotreviewbot

Col: 14 E111 indentation is not a multiple of four

reviewbotreviewbot

Col: 14 E111 indentation is not a multiple of four

reviewbotreviewbot

Col: 54 E201 whitespace after '('

reviewbotreviewbot

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

reviewbotreviewbot

'os' imported but unused

reviewbotreviewbot

'make_tempfile' imported but unused

reviewbotreviewbot

Col: 13 E265 block comment should start with '# '

reviewbotreviewbot

Col: 1 W293 blank line contains whitespace

reviewbotreviewbot

Col: 14 E111 indentation is not a multiple of four

reviewbotreviewbot

Col: 14 E111 indentation is not a multiple of four

reviewbotreviewbot

Col: 54 E201 whitespace after '('

reviewbotreviewbot

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

reviewbotreviewbot

Col: 1 W293 blank line contains whitespace

reviewbotreviewbot

Col: 14 E111 indentation is not a multiple of four

reviewbotreviewbot

Col: 14 E111 indentation is not a multiple of four

reviewbotreviewbot

Couple of things here: - The first line of this docstring should describe what the class is, instead of stating ...

anselinaanselina

This does not return anything. It is a class.

brenniebrennie

Remove this line.

anselinaanselina

Since ShellCheck also identifies potential failures in addition to just style errors, let's update the description to: Checks shell scripts ...

anselinaanselina

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

anselinaanselina

Comments should be written in full sentences, like: # Ignore the file if it is not a shell script.

anselinaanselina

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

anselinaanselina

It'd be useful to add a comment above this line showing an example of shellcheck's output.

anselinaanselina

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

anselinaanselina

Col: 14 E111 indentation is not a multiple of four

reviewbotreviewbot

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

anselinaanselina

Col: 14 E111 indentation is not a multiple of four

reviewbotreviewbot

Blank line between these.

brenniebrennie

Col: 80 E501 line too long (94 > 79 characters)

reviewbotreviewbot

This string should end with a space.

brenniebrennie

Col: 82 W291 trailing whitespace

reviewbotreviewbot

Col: 80 E501 line too long (81 > 79 characters)

reviewbotreviewbot

Likewise.

brenniebrennie

Col: 83 W291 trailing whitespace

reviewbotreviewbot

Col: 80 E501 line too long (82 > 79 characters)

reviewbotreviewbot

But this one should not.

brenniebrennie

Col: 80 E501 line too long (91 > 79 characters)

reviewbotreviewbot

This should be a ChoiceField, not a CharField

brenniebrennie

Col: 80 E501 line too long (86 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (81 > 79 characters)

reviewbotreviewbot

Col: 34 W291 trailing whitespace

reviewbotreviewbot

Remove this comment.

brenniebrennie

This would be good in the docstring of the function.

brenniebrennie

This can all be on one line.

brenniebrennie

Col: 14 E111 indentation is not a multiple of four

reviewbotreviewbot

Col: 14 E111 indentation is not a multiple of four

reviewbotreviewbot

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

brenniebrennie

Col: 65 W291 trailing whitespace

reviewbotreviewbot

Col: 1 W293 blank line contains whitespace

reviewbotreviewbot

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

reviewbotreviewbot

Col: 35 W291 trailing whitespace

reviewbotreviewbot

Col: 32 E225 missing whitespace around operator

reviewbotreviewbot

Col: 33 E225 missing whitespace around operator

reviewbotreviewbot

Col: 14 E111 indentation is not a multiple of four

reviewbotreviewbot

Col: 14 E111 indentation is not a multiple of four

reviewbotreviewbot

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

reviewbotreviewbot

Col: 14 E111 indentation is not a multiple of four

reviewbotreviewbot

Col: 14 E111 indentation is not a multiple of four

reviewbotreviewbot

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

reviewbotreviewbot

Col: 44 W291 trailing whitespace

reviewbotreviewbot

Col: 46 W291 trailing whitespace

reviewbotreviewbot

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

reviewbotreviewbot

Col: 49 W291 trailing whitespace

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

Col: 19 E124 closing bracket does not match visual indentation

reviewbotreviewbot

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

reviewbotreviewbot

Col: 17 E124 closing bracket does not match visual indentation

reviewbotreviewbot

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

reviewbotreviewbot

Col: 17 E124 closing bracket does not match visual indentation

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

Col: 13 E124 closing bracket does not match visual indentation

reviewbotreviewbot

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

reviewbotreviewbot

Col: 19 E124 closing bracket does not match visual indentation

reviewbotreviewbot

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

reviewbotreviewbot

Col: 17 E124 closing bracket does not match visual indentation

reviewbotreviewbot

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

reviewbotreviewbot

Col: 17 E124 closing bracket does not match visual indentation

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

This doesn't belong in the docstring of the tool.

brenniebrennie

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

reviewbotreviewbot

These lines should be indented 4 relative to description.

brenniebrennie

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

brenniebrennie

Should only be one space after "Diagnostic". Also, what if we want to ignore more than one error? This should ...

brenniebrennie

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

brenniebrennie

Are we absolutely certain that we don't want to get JSON output from the shellcheck tool, parse it, and iterate ...

mike_conleymike_conley

Re: above comments, this should check for a comma-separated list in this setting and loop through them. Shellcheck may allow ...

brenniebrennie

Format this as: output = execute(command_line, split_lines=True, ignore_errors=True)

brenniebrennie

Blank line between these.

brenniebrennie

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

brenniebrennie

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

reviewbotreviewbot

Col: 26 E201 whitespace after '('

reviewbotreviewbot

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

reviewbotreviewbot

Col: 80 E501 line too long (80 > 79 characters)

reviewbotreviewbot

Col: 52 W291 trailing whitespace

reviewbotreviewbot

Col: 80 E501 line too long (80 > 79 characters)

reviewbotreviewbot

Col: 52 W291 trailing whitespace

reviewbotreviewbot

Col: 30 W291 trailing whitespace

reviewbotreviewbot

Col: 34 W291 trailing whitespace

reviewbotreviewbot

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

daviddavid

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

daviddavid

This should probably be a comment, not a docstring.

daviddavid

Can we reformat this as such: command_line.append('--format=%s' % self.settings['specify_dialect'])

daviddavid

Same here.

daviddavid

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

daviddavid

This is a very poor error message.

daviddavid

Col: 50 W291 trailing whitespace

reviewbotreviewbot

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

reviewbotreviewbot

Col: 51 W291 trailing whitespace

reviewbotreviewbot

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

reviewbotreviewbot

Col: 52 W292 no newline at end of file

reviewbotreviewbot

Col: 50 W291 trailing whitespace

reviewbotreviewbot

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

reviewbotreviewbot

Col: 51 W291 trailing whitespace

reviewbotreviewbot

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

reviewbotreviewbot

Col: 52 W292 no newline at end of file

reviewbotreviewbot

You should mention this accepts comma separated list.

brenniebrennie

This is not useful. Remove the try: block.

brenniebrennie

Col: 20 W292 no newline at end of file

reviewbotreviewbot
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        bot/reviewbot/tools/shellcheck.py
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     E265 block comment should start with '# '
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     E265 block comment should start with '# '
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  7. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  8. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  9. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  10. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 2
     E265 block comment should start with '# '
    
  11. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  12. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  13. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  14. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  15. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  16. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 9
     E112 expected an indented block
    
  17. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  18. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 9
     E265 block comment should start with '# '
    
  19. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  20. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (85 > 79 characters)
    
  21. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (91 > 79 characters)
    
  22. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (95 > 79 characters)
    
  23. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (89 > 79 characters)
    
  24. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (82 > 79 characters)
    
  25. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (107 > 79 characters)
    
  26. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 25
     W291 trailing whitespace
    
  27. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 57
     W291 trailing whitespace
    
  28. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  29. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  30. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
    Col: 20
     W291 trailing whitespace
    
  31. 
      
XU
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        bot/reviewbot/tools/shellcheck.py
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 1
     E265 block comment should start with '# '
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 1
     E265 block comment should start with '# '
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 1
     W191 indentation contains tabs
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 1
     W191 indentation contains tabs
    
  7. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 1
     W191 indentation contains tabs
    
  8. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 1
     W191 indentation contains tabs
    
  9. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 1
     W191 indentation contains tabs
    
  10. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 2
     E265 block comment should start with '# '
    
  11. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 1
     W191 indentation contains tabs
    
  12. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 1
     W191 indentation contains tabs
    
  13. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 1
     W191 indentation contains tabs
    
  14. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 1
     W191 indentation contains tabs
    
  15. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  16. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 9
     E112 expected an indented block
    
  17. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 9
     E265 block comment should start with '# '
    
  18. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (85 > 79 characters)
    
  19. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (91 > 79 characters)
    
  20. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (95 > 79 characters)
    
  21. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (89 > 79 characters)
    
  22. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (82 > 79 characters)
    
  23. bot/reviewbot/tools/shellcheck.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (107 > 79 characters)
    
  24. 
      
XU
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        bot/reviewbot/tools/shellcheck.py
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 3)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 3)
     
     
    Col: 1
     W191 indentation contains tabs
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 3)
     
     
    Col: 1
     W191 indentation contains tabs
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 3)
     
     
    Col: 1
     W191 indentation contains tabs
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 3)
     
     
    Col: 1
     W191 indentation contains tabs
    
  7. bot/reviewbot/tools/shellcheck.py (Diff revision 3)
     
     
    Col: 1
     W191 indentation contains tabs
    
  8. bot/reviewbot/tools/shellcheck.py (Diff revision 3)
     
     
    Col: 2
     E303 too many blank lines (2)
    
  9. bot/reviewbot/tools/shellcheck.py (Diff revision 3)
     
     
    Col: 1
     W191 indentation contains tabs
    
  10. bot/reviewbot/tools/shellcheck.py (Diff revision 3)
     
     
    Col: 1
     W191 indentation contains tabs
    
  11. bot/reviewbot/tools/shellcheck.py (Diff revision 3)
     
     
    Col: 1
     W191 indentation contains tabs
    
  12. bot/reviewbot/tools/shellcheck.py (Diff revision 3)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  13. bot/reviewbot/tools/shellcheck.py (Diff revision 3)
     
     
    Col: 9
     E112 expected an indented block
    
  14. bot/reviewbot/tools/shellcheck.py (Diff revision 3)
     
     
    Col: 9
     E265 block comment should start with '# '
    
  15. bot/reviewbot/tools/shellcheck.py (Diff revision 3)
     
     
    Col: 20
     W292 no newline at end of file
    
  16. 
      
XU
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        bot/reviewbot/tools/shellcheck.py
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 4)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 4)
     
     
    Col: 2
     E303 too many blank lines (2)
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 4)
     
     
    Col: 2
     E113 unexpected indentation
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 4)
     
     
    Col: 1
     W191 indentation contains tabs
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 4)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  7. bot/reviewbot/tools/shellcheck.py (Diff revision 4)
     
     
    Col: 9
     E112 expected an indented block
    
  8. bot/reviewbot/tools/shellcheck.py (Diff revision 4)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  9. bot/reviewbot/tools/shellcheck.py (Diff revision 4)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  10. bot/reviewbot/tools/shellcheck.py (Diff revision 4)
     
     
    Col: 1
     W191 indentation contains tabs
    
  11. bot/reviewbot/tools/shellcheck.py (Diff revision 4)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  12. bot/reviewbot/tools/shellcheck.py (Diff revision 4)
     
     
    Col: 20
     W292 no newline at end of file
    
  13. 
      
XU
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        bot/reviewbot/tools/shellcheck.py
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 5)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 5)
     
     
    Col: 5
     E112 expected an indented block
    
  4. 
      
XU
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        bot/reviewbot/tools/shellcheck.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        bot/reviewbot/tools/shellcheck.py
    
    
  2. 
      
mike_conley
  1. 
      
  2. 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?

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

    2. Can you add "WIP" to the summary line?

  3. 
      
XU
chipx86
  1. 
      
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 6)
     
     

    Needs a docstring.

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

    "script"

    Missing period.

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

  5. 
      
david
  1. 
      
  2. 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.

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

  3. 
      
anselina
  1. 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.

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

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

    Remove this comment since the code is self-explanatory.

  4. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 7)
     
     
    Col: 67
     W291 trailing whitespace
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 7)
     
     
    Col: 13
     E265 block comment should start with '# '
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 7)
     
     
    Col: 13
     E265 block comment should start with '# '
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 7)
     
     
    Col: 13
     E265 block comment should start with '# '
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 7)
     
     
    Col: 13
     E265 block comment should start with '# '
    
  7. bot/setup.py (Diff revision 7)
     
     
    Col: 70
     W291 trailing whitespace
    
  8. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 8)
     
     
    Col: 67
     W291 trailing whitespace
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 8)
     
     
    Col: 13
     E265 block comment should start with '# '
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 8)
     
     
    Col: 13
     E265 block comment should start with '# '
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 8)
     
     
    Col: 13
     E265 block comment should start with '# '
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 8)
     
     
    Col: 13
     E265 block comment should start with '# '
    
  7. bot/setup.py (Diff revision 8)
     
     
    Col: 70
     W291 trailing whitespace
    
  8. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 9)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 9)
     
     
    Col: 61
     W291 trailing whitespace
    
  4. 
      
XU
reviewbot
  1. 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
    
    
  2. 
      
anselina
  1. Looks like you should be ready to test your shellcheck tool this week!

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

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

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

    s/Ruturn/Return

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

    Add a blank line before this line.

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

  6. 
      
XU
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        bot/reviewbot/tools/shellcheck.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        bot/reviewbot/tools/shellcheck.py
    
    
  2. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/buildbot.py (Diff revision 12)
     
     
     'os' imported but unused
    
  3. bot/reviewbot/tools/buildbot.py (Diff revision 12)
     
     
     'make_tempfile' imported but unused
    
  4. bot/reviewbot/tools/buildbot.py (Diff revision 12)
     
     
    Col: 13
     E265 block comment should start with '# '
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 12)
     
     
    Col: 14
     E111 indentation is not a multiple of four
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 12)
     
     
    Col: 14
     E111 indentation is not a multiple of four
    
  7. 
      
david
  1. 
      
  2. 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 of File_name).

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

  3. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/buildbot.py (Diff revision 13)
     
     
     'os' imported but unused
    
  3. bot/reviewbot/tools/buildbot.py (Diff revision 13)
     
     
     'make_tempfile' imported but unused
    
  4. bot/reviewbot/tools/buildbot.py (Diff revision 13)
     
     
    Col: 13
     E265 block comment should start with '# '
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 13)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 13)
     
     
    Col: 14
     E111 indentation is not a multiple of four
    
  7. bot/reviewbot/tools/shellcheck.py (Diff revision 13)
     
     
    Col: 14
     E111 indentation is not a multiple of four
    
  8. bot/reviewbot/tools/shellcheck.py (Diff revision 13)
     
     
    Col: 54
     E201 whitespace after '('
    
  9. bot/reviewbot/tools/shellcheck.py (Diff revision 13)
     
     
    Col: 17
     E128 continuation line under-indented for visual indent
    
  10. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/buildbot.py (Diff revision 14)
     
     
     'os' imported but unused
    
  3. bot/reviewbot/tools/buildbot.py (Diff revision 14)
     
     
     'make_tempfile' imported but unused
    
  4. bot/reviewbot/tools/buildbot.py (Diff revision 14)
     
     
    Col: 13
     E265 block comment should start with '# '
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 14)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 14)
     
     
    Col: 14
     E111 indentation is not a multiple of four
    
  7. bot/reviewbot/tools/shellcheck.py (Diff revision 14)
     
     
    Col: 14
     E111 indentation is not a multiple of four
    
  8. bot/reviewbot/tools/shellcheck.py (Diff revision 14)
     
     
    Col: 54
     E201 whitespace after '('
    
  9. bot/reviewbot/tools/shellcheck.py (Diff revision 14)
     
     
    Col: 17
     E128 continuation line under-indented for visual indent
    
  10. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 15)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 15)
     
     
    Col: 14
     E111 indentation is not a multiple of four
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 15)
     
     
    Col: 14
     E111 indentation is not a multiple of four
    
  5. 
      
XU
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 16)
     
     
    Col: 14
     E111 indentation is not a multiple of four
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 16)
     
     
    Col: 14
     E111 indentation is not a multiple of four
    
  4. 
      
XU
anselina
  1. 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?

  2. 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 the handle_file method).

    I think the docstring for this can be pretty simple, really (maybe just """ShellCheck tool plugin implementation.""").

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

    Remove this line.

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

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

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

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

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

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

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

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

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

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

  11. 
      
brennie
  1. 
      
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 16)
     
     

    This does not return anything. It is a class.

  3. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 17)
     
     
    Col: 80
     E501 line too long (94 > 79 characters)
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 17)
     
     
    Col: 82
     W291 trailing whitespace
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 17)
     
     
    Col: 80
     E501 line too long (81 > 79 characters)
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 17)
     
     
    Col: 83
     W291 trailing whitespace
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 17)
     
     
    Col: 80
     E501 line too long (82 > 79 characters)
    
  7. bot/reviewbot/tools/shellcheck.py (Diff revision 17)
     
     
    Col: 80
     E501 line too long (91 > 79 characters)
    
  8. bot/reviewbot/tools/shellcheck.py (Diff revision 17)
     
     
    Col: 80
     E501 line too long (86 > 79 characters)
    
  9. bot/reviewbot/tools/shellcheck.py (Diff revision 17)
     
     
    Col: 80
     E501 line too long (81 > 79 characters)
    
  10. bot/reviewbot/tools/shellcheck.py (Diff revision 17)
     
     
    Col: 34
     W291 trailing whitespace
    
  11. bot/reviewbot/tools/shellcheck.py (Diff revision 17)
     
     
    Col: 14
     E111 indentation is not a multiple of four
    
  12. bot/reviewbot/tools/shellcheck.py (Diff revision 17)
     
     
    Col: 14
     E111 indentation is not a multiple of four
    
  13. 
      
brennie
  1. 
      
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 17)
     
     
     

    Blank line between these.

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

    This string should end with a space.

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

    Likewise.

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

    But this one should not.

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

    This should be a ChoiceField, not a CharField

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

    Remove this comment.

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

    This would be good in the docstring of the function.

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

    This can all be on one line.

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

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

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

  11. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 18)
     
     
    Col: 65
     W291 trailing whitespace
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 18)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 18)
     
     
    Col: 19
     E128 continuation line under-indented for visual indent
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 18)
     
     
    Col: 35
     W291 trailing whitespace
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 18)
     
     
    Col: 32
     E225 missing whitespace around operator
    
  7. bot/reviewbot/tools/shellcheck.py (Diff revision 18)
     
     
    Col: 33
     E225 missing whitespace around operator
    
  8. bot/reviewbot/tools/shellcheck.py (Diff revision 18)
     
     
    Col: 14
     E111 indentation is not a multiple of four
    
  9. bot/reviewbot/tools/shellcheck.py (Diff revision 18)
     
     
    Col: 14
     E111 indentation is not a multiple of four
    
  10. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 19)
     
     
    Col: 19
     E128 continuation line under-indented for visual indent
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 19)
     
     
    Col: 14
     E111 indentation is not a multiple of four
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 19)
     
     
    Col: 14
     E111 indentation is not a multiple of four
    
  5. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 20)
     
     
    Col: 19
     E128 continuation line under-indented for visual indent
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 20)
     
     
    Col: 44
     W291 trailing whitespace
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 20)
     
     
    Col: 46
     W291 trailing whitespace
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 20)
     
     
    Col: 17
     E128 continuation line under-indented for visual indent
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 20)
     
     
    Col: 49
     W291 trailing whitespace
    
  7. bot/reviewbot/tools/shellcheck.py (Diff revision 20)
     
     
    Col: 17
     E128 continuation line under-indented for visual indent
    
  8. bot/reviewbot/tools/shellcheck.py (Diff revision 20)
     
     
    Col: 13
     E128 continuation line under-indented for visual indent
    
  9. bot/reviewbot/tools/shellcheck.py (Diff revision 20)
     
     
    Col: 13
     E128 continuation line under-indented for visual indent
    
  10. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 21)
     
     
    Col: 19
     E128 continuation line under-indented for visual indent
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 21)
     
     
    Col: 19
     E124 closing bracket does not match visual indentation
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 21)
     
     
    Col: 17
     E128 continuation line under-indented for visual indent
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 21)
     
     
    Col: 17
     E124 closing bracket does not match visual indentation
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 21)
     
     
    Col: 17
     E128 continuation line under-indented for visual indent
    
  7. bot/reviewbot/tools/shellcheck.py (Diff revision 21)
     
     
    Col: 17
     E124 closing bracket does not match visual indentation
    
  8. bot/reviewbot/tools/shellcheck.py (Diff revision 21)
     
     
    Col: 13
     E128 continuation line under-indented for visual indent
    
  9. bot/reviewbot/tools/shellcheck.py (Diff revision 21)
     
     
    Col: 13
     E128 continuation line under-indented for visual indent
    
  10. bot/reviewbot/tools/shellcheck.py (Diff revision 21)
     
     
    Col: 13
     E124 closing bracket does not match visual indentation
    
  11. 
      
XU
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 22)
     
     
    Col: 19
     E128 continuation line under-indented for visual indent
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 22)
     
     
    Col: 19
     E124 closing bracket does not match visual indentation
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 22)
     
     
    Col: 17
     E128 continuation line under-indented for visual indent
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 22)
     
     
    Col: 17
     E124 closing bracket does not match visual indentation
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 22)
     
     
    Col: 17
     E128 continuation line under-indented for visual indent
    
  7. bot/reviewbot/tools/shellcheck.py (Diff revision 22)
     
     
    Col: 17
     E124 closing bracket does not match visual indentation
    
  8. bot/reviewbot/tools/shellcheck.py (Diff revision 22)
     
     
    Col: 13
     E128 continuation line under-indented for visual indent
    
  9. bot/reviewbot/tools/shellcheck.py (Diff revision 22)
     
     
    Col: 13
     E128 continuation line under-indented for visual indent
    
  10. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 23)
     
     
    Col: 19
     E126 continuation line over-indented for hanging indent
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 23)
     
     
    Col: 19
     E123 closing bracket does not match indentation of opening bracket's line
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 23)
     
     
    Col: 17
     E123 closing bracket does not match indentation of opening bracket's line
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 23)
     
     
    Col: 17
     E123 closing bracket does not match indentation of opening bracket's line
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 23)
     
     
    Col: 13
     E123 closing bracket does not match indentation of opening bracket's line
    
  7. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 24)
     
     
    Col: 19
     E126 continuation line over-indented for hanging indent
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 24)
     
     
    Col: 19
     E123 closing bracket does not match indentation of opening bracket's line
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 24)
     
     
    Col: 32
     E126 continuation line over-indented for hanging indent
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 24)
     
     
    Col: 32
     E126 continuation line over-indented for hanging indent
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 24)
     
     
    Col: 24
     E126 continuation line over-indented for hanging indent
    
  7. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 25)
     
     
    Col: 19
     E128 continuation line under-indented for visual indent
    
  3. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 26)
     
     
    Col: 19
     E126 continuation line over-indented for hanging indent
    
  3. 
      
brennie
  1. 
      
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 26)
     
     
     
     

    This doesn't belong in the docstring of the tool.

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

    These lines should be indented 4 relative to description.

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

    2.     description = (
              'Checks shell scripts for ...'
              'potential ...')
      
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 26)
     
     

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

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

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

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

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

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

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

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

    Format this as:

    output = execute(command_line, split_lines=True, ignore_errors=True)
    
  9. bot/reviewbot/tools/shellcheck.py (Diff revision 26)
     
     
     

    Blank line between these.

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

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

  11. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 27)
     
     
    Col: 19
     E126 continuation line over-indented for hanging indent
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 27)
     
     
    Col: 26
     E201 whitespace after '('
    
  4. 
      
mike_conley
  1. 
      
  2. 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.

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

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

  3. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 28)
     
     
    Col: 19
     E126 continuation line over-indented for hanging indent
    
  3. 
      
XU
reviewbot
  1. 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
    
    
  2. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 30)
     
     
    Col: 80
     E501 line too long (80 > 79 characters)
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 30)
     
     
    Col: 52
     W291 trailing whitespace
    
  4. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 31)
     
     
    Col: 80
     E501 line too long (80 > 79 characters)
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 31)
     
     
    Col: 52
     W291 trailing whitespace
    
  4. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 32)
     
     
    Col: 30
     W291 trailing whitespace
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 32)
     
     
    Col: 34
     W291 trailing whitespace
    
  4. 
      
XU
reviewbot
  1. 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
    
    
  2. 
      
david
  1. What's the issue with testing and options?

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

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

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

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

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

    This should probably be a comment, not a docstring.

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

    Can we reformat this as such:

    command_line.append('--format=%s'
                        % self.settings['specify_dialect'])
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 33)
     
     
     

    Same here.

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

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

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

    This is a very poor error message.

  9. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 34)
     
     
    Col: 50
     W291 trailing whitespace
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 34)
     
     
    Col: 38
     E127 continuation line over-indented for visual indent
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 34)
     
     
    Col: 51
     W291 trailing whitespace
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 34)
     
     
    Col: 38
     E127 continuation line over-indented for visual indent
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 34)
     
     
    Col: 52
     W292 no newline at end of file
    
  7. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 35)
     
     
    Col: 50
     W291 trailing whitespace
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 35)
     
     
    Col: 38
     E127 continuation line over-indented for visual indent
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 35)
     
     
    Col: 51
     W291 trailing whitespace
    
  5. bot/reviewbot/tools/shellcheck.py (Diff revision 35)
     
     
    Col: 38
     E127 continuation line over-indented for visual indent
    
  6. bot/reviewbot/tools/shellcheck.py (Diff revision 35)
     
     
    Col: 52
     W292 no newline at end of file
    
  7. 
      
XU
reviewbot
  1. 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
    
    
  2. 
      
XU
reviewbot
  1. 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
    
    
  2. 
      
brennie
  1. 
      
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 37)
     
     
     

    You should mention this accepts comma separated list.

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

    This is not useful. Remove the try: block.

  4. 
      
XU
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        bot/reviewbot/tools/shellcheck.py
        bot/setup.py
    
    
  2. 
      
XU
XU
XU
reviewbot
  1. 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
    
    
  2. 
      
XU
reviewbot
  1. 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
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 40)
     
     
    Col: 20
     W292 no newline at end of file
    
  3. 
      
XU
reviewbot
  1. 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
    
    
  2. 
      
XU
Review request changed

Status: Discarded

Loading...