[WIP]ReviewBot ShellCheckTool

Review Request #7197 — Created April 12, 2015 and discarded

Information

ReviewBot
release-0.2.x

Reviewers

ReviewBot ShellCheckTool

apt-get install cabal-install
cabal update
cabal install shellcheck
ln -s ~/.cabal/bin/shellcheck ~/usr/local/bin/shellcheck

Description From Last Updated

We don't use module-level docstrings.

brenniebrennie

We organize our imports into three groups Python standard library imports Third party imports Project imports Given that, can we …

brenniebrennie

"tool" not "model".

brenniebrennie

Single quotes around sh.

brenniebrennie

This sentence is implied by the first.

brenniebrennie

A few grammar mistakes here, how about: """Use ShellCheck to analyze the file and post the result."""

brenniebrennie

Blank line between end of block and statement.

brenniebrennie

Blank line between statement and block.

brenniebrennie

Blank line between these.

brenniebrennie

Can we pull this out into a variable like shellcheck_cmd and then we can format the execute like: output = …

brenniebrennie

You probably want none_on_ignored_error=True as well.

brenniebrennie

Blank line between statement and block.

brenniebrennie

What is cmm? This needs a better name.

brenniebrennie

Blank line between statement and block.

brenniebrennie

Can you include some sample output of shellcheck?

brenniebrennie

Blank line between statement and block. Also, can you make this regular expression a compiled constant on the class using …

brenniebrennie

Can you also compile this? Please document what it does also.

brenniebrennie

Blank line between two blocks. If we are expecting a lot of strings to join, we should make cmm an …

brenniebrennie

This should not be included in your change.

brenniebrennie
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. 
      
brennie
  1. 
      
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     
     

    We don't use module-level docstrings.

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

    We organize our imports into three groups

    1. Python standard library imports
    2. Third party imports
    3. Project imports

    Given that, can we reformat these imports as

    import re
    
    from reviewbot.tools import Tool
    # ...
    
  4. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     

    "tool" not "model".

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

    Single quotes around sh.

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

    This sentence is implied by the first.

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

    A few grammar mistakes here, how about:

    """Use ShellCheck to analyze the file and post the result."""

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

    Blank line between end of block and statement.

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

    Blank line between statement and block.

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

    Blank line between these.

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

    Can we pull this out into a variable like shellcheck_cmd and then we can format the execute like:

    output = execute(shellcheck_cmd, split_lines=True, ignore_errors=True)
    
  12. bot/reviewbot/tools/shellcheck.py (Diff revision 1)
     
     

    You probably want none_on_ignored_error=True as well.

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

    Blank line between statement and block.

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

    What is cmm? This needs a better name.

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

    Blank line between statement and block.

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

    Can you include some sample output of shellcheck?

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

    Blank line between statement and block.

    Also, can you make this regular expression a compiled constant on the class using re.compile? Please document what it does also.

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

    Can you also compile this? Please document what it does also.

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

    Blank line between two blocks.

    If we are expecting a lot of strings to join, we should make cmm an array and then use ''.join(cmm) when we go to comment.

  20. bot/setup.py (Diff revision 1)
     
     

    This should not be included in your change.

  21. 
      
CA
Review request changed

Status: Discarded

Change Summary:

Discarded in favour of https://reviews.reviewboard.org/r/7701.

Loading...