[WIP]ReviewBot ShellCheckTool

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

Caster.Rune
ReviewBot
release-0.2.x
reviewbot, students

ReviewBot ShellCheckTool

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

  • 19
  • 0
  • 0
  • 0
  • 19
Description From Last Updated
We don't use module-level docstrings. brennie brennie
We organize our imports into three groups Python standard library imports Third party imports Project imports Given that, can we ... brennie brennie
"tool" not "model". brennie brennie
Single quotes around sh. brennie brennie
This sentence is implied by the first. brennie brennie
A few grammar mistakes here, how about: """Use ShellCheck to analyze the file and post the result.""" brennie brennie
Blank line between end of block and statement. brennie brennie
Blank line between statement and block. brennie brennie
Blank line between these. brennie brennie
Can we pull this out into a variable like shellcheck_cmd and then we can format the execute like: output = ... brennie brennie
You probably want none_on_ignored_error=True as well. brennie brennie
Blank line between statement and block. brennie brennie
What is cmm? This needs a better name. brennie brennie
Blank line between statement and block. brennie brennie
Can you include some sample output of shellcheck? brennie brennie
Blank line between statement and block. Also, can you make this regular expression a compiled constant on the class using ... brennie brennie
Can you also compile this? Please document what it does also. brennie brennie
Blank line between two blocks. If we are expecting a lot of strings to join, we should make cmm an ... brennie brennie
This should not be included in your change. brennie brennie
  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. 
      
Review request changed

Status: Discarded

Change Summary:

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

Loading...