[WIP]ReviewBot ShellCheckTool
Review Request #7197 — Created April 12, 2015 and discarded
| Information | |
|---|---|
| Caster.Rune | |
| ReviewBot | |
| release-0.2.x | |
| Reviewers | |
| reviewbot, students | |
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. |
 brennie
|
|
|
We organize our imports into three groups Python standard library imports Third party imports Project imports Given that, can we … |
brennie
|
|
|
"tool" not "model". |
brennie
|
|
|
Single quotes around sh. |
brennie
|
|
|
This sentence is implied by the first. |
brennie
|
|
|
A few grammar mistakes here, how about: """Use ShellCheck to analyze the file and post the result.""" |
brennie
|
|
|
Blank line between end of block and statement. |
brennie
|
|
|
Blank line between statement and block. |
brennie
|
|
|
Blank line between these. |
brennie
|
|
|
Can we pull this out into a variable like shellcheck_cmd and then we can format the execute like: output = … |
brennie
|
|
|
You probably want none_on_ignored_error=True as well. |
brennie
|
|
|
Blank line between statement and block. |
brennie
|
|
|
What is cmm? This needs a better name. |
brennie
|
|
|
Blank line between statement and block. |
brennie
|
|
|
Can you include some sample output of shellcheck? |
brennie
|
|
|
Blank line between statement and block. Also, can you make this regular expression a compiled constant on the class using … |
brennie
|
|
|
Can you also compile this? Please document what it does also. |
brennie
|
|
|
Blank line between two blocks. If we are expecting a lot of strings to join, we should make cmm an … |
brennie
|
|
|
This should not be included in your change. |
brennie
|
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 1) We organize our imports into three groups
- Python standard library imports
- Third party imports
- Project imports
Given that, can we reformat these imports as
import re from reviewbot.tools import Tool # ... -
-
-
-
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."""
-
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 1) Can we pull this out into a variable like
shellcheck_cmdand then we can format the execute like:output = execute(shellcheck_cmd, split_lines=True, ignore_errors=True) -
bot/reviewbot/tools/shellcheck.py (Diff revision 1) You probably want
none_on_ignored_error=Trueas well. -
-
-
-
bot/reviewbot/tools/shellcheck.py (Diff revision 1) Can you include some sample output of shellcheck?
-
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. -
bot/reviewbot/tools/shellcheck.py (Diff revision 1) Can you also compile this? Please document what it does also.
-
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
cmman array and then use''.join(cmm)when we go to comment. -
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