flake8
-
bot/reviewbot/tools/go.py (Diff revision 1) Show all issues
Review Request #10302 — Created Nov. 1, 2018 and discarded
Information | |
---|---|
ammar | |
ReviewBot | |
master | |
|
|
Reviewers | |
reviewbot, students | |
This patch adds a Go Tool to Review Bot for static analysis of Go code.
It makes use of the following two Go commands:
go tool vet
# static code analysis to find possible bugs or suspicious
constructs
gofmt
# formats the code
Manual tests. Successfuly adds issues at the right line numbers.
Description | From | Last Updated |
---|---|---|
E999 SyntaxError: EOL while scanning string literal |
![]() |
|
F841 local variable 'vet_output' is assigned to but never used |
![]() |
|
F821 undefined name 'output' |
![]() |
|
Single quotes. |
|
|
No blank line here. |
|
|
We might want to make a note that we are using gofmt over go fmt because the former does not … |
|
|
In the case of a string that contains single quotes, it is more readable to use double quotes, e.g. "%s's … |
|
|
No blank line here. |
|
|
I think this is a typo because line.split(':', 1) will always two values if : is in line. Should this … |
|
|
This can all be one line. |
|
|
Instead of ignoring all errors, how about just extra_ignore_errors=(2,) |
|
|
This will put all of the "options" into a single argument. Do we want to use settings['vet_options'].split()? |
|
|
Slightly better to do vet_args_list.append(path) |
|
|
The number of = characters should line up with the number of characters in the title: ======= Go Tool ======= |
|
|
Add an additional blank line here. |
|
|
The "note" is probably not necessary. |
|
|
The 'go' between run and go vet can be removed? |
|
|
Should this be using single quotes? |
![]() |
|
E202 whitespace before ']' |
![]() |
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+92) |
bot/reviewbot/tools/go.py (Diff revision 2) |
---|
F841 local variable 'vet_output' is assigned to but never used
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+92) |
bot/reviewbot/tools/go.py (Diff revision 3) |
---|
We might want to make a note that we are using
gofmt
overgo fmt
because the former does not allow us to specify options that we want to use.
bot/reviewbot/tools/go.py (Diff revision 3) |
---|
In the case of a string that contains single quotes, it is more readable to use double quotes, e.g.
"%s's formatting differs from gofmt's"
bot/reviewbot/tools/go.py (Diff revision 3) |
---|
I think this is a typo because
line.split(':', 1)
will always two values if:
is in line. Should this sayIf split does not return two values
?Also this should be in the except block.
Comments must be proper sentences.
Description: |
|
||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
||||||||||||||||||||||||||||||||||||
Commit: |
|
||||||||||||||||||||||||||||||||||||
Diff: |
Revision 4 (+123) |
bot/reviewbot/tools/go.py (Diff revision 4) |
---|
Instead of ignoring all errors, how about just
extra_ignore_errors=(2,)
bot/reviewbot/tools/go.py (Diff revision 4) |
---|
This will put all of the "options" into a single argument. Do we want to use
settings['vet_options'].split()
?
docs/reviewbot/tools/gotool.rst (Diff revision 4) |
---|
The number of
=
characters should line up with the number of characters in the title:======= Go Tool =======
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 5 (+123) |
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 6 (+123) |
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 7 (+119) |
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 8 (+119) |