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