Replacing non-named usages to use names logging in files in reviewboard.notifications
Review Request #11396 — Created Jan. 22, 2021 and submitted
Information | |
---|---|
amohapatra | |
Review Board | |
master | |
4907 | |
Reviewers | |
reviewboard, students | |
This commit is replacing non-named usages in reviewboard/notifications to use
named logging instead. The purpose of this change is to automatically include
more information about the module where the logging message came from so that
the messages can be of more use when diagnosing problems from logs.
I ran the unit tests under reviewboard/notifications/tests by running
./tests/runtests.py reviewboard.notifications.tests
and confirmed that all
162 tests ran without fail.
Summary | Author |
---|---|
anahita-m | |
anahita-m |
Description | From | Last Updated |
---|---|---|
Can you wrap your description and testing done to be 80 characters in width? |
|
|
If you are only importing logger from reviewboard.notifications.webhooks shouldn't you import it once at the top of the file? |
|
|
Please put that second blank line back. |
|
|
Each of these blank lines should be two. |
|
|
Please put this blank line back. |
|
|
We don't need a blank line here. |
|
|
This blank line can go away. |
|
|
Please add another blank line here. |
|
|
E101 indentation contains mixed spaces and tabs |
![]() |
|
W191 indentation contains tabs |
![]() |
|
E128 continuation line under-indented for visual indent |
![]() |
|
E101 indentation contains mixed spaces and tabs |
![]() |
|
E128 continuation line under-indented for visual indent |
![]() |
|
E231 missing whitespace after ',' |
![]() |
|
E128 continuation line under-indented for visual indent |
![]() |
|
E101 indentation contains mixed spaces and tabs |
![]() |
|
W191 indentation contains tabs |
![]() |
|
W191 indentation contains tabs |
![]() |
|
E101 indentation contains mixed spaces and tabs |
![]() |
|
E128 continuation line under-indented for visual indent |
![]() |
|
E101 indentation contains mixed spaces and tabs |
![]() |
|
E128 continuation line under-indented for visual indent |
![]() |
|
Indentation will need to be fixed here, since logger is one character shorter than logging. Please check all your changed … |
|
|
Please revert the changes to these lines. |
|
|
These lines need to be dedented one space, because "logger" is one character shorter than "logging". |
|
|
Dedent one space. |
|
|
Dedent one space. |
|
|
Dedent one space. |
|
|
Dedent one space. |
|
|
Dedent one space. |
|
|
Dedent one space. |
|
Testing Done: |
|
---|
-
-
reviewboard/notifications/tests/test_webhooks.py (Diff revision 1) If you are only importing logger from
reviewboard.notifications.webhooks
shouldn't you import it once at the top of the file?
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+76 -64) |
Checks run (2 succeeded)
-
This looks pretty good so far. I have a bunch of comments but they all relate to the placement of blank lines. The most confusing part of it is that Python specifies that at the top-level scope there should be two blank lines between definitions.
-
-
reviewboard/notifications/email/message.py (Diff revision 2) Please put that second blank line back.
-
-
-
-
-
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+98 -72) |
Checks run (1 failed, 1 succeeded)
flake8
-
reviewboard/notifications/email/message.py (Diff revision 3) E101 indentation contains mixed spaces and tabs
-
-
reviewboard/notifications/email/message.py (Diff revision 3) E128 continuation line under-indented for visual indent
-
reviewboard/notifications/email/message.py (Diff revision 3) E101 indentation contains mixed spaces and tabs
-
reviewboard/notifications/email/message.py (Diff revision 3) E128 continuation line under-indented for visual indent
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 4 (+96 -72) |
Checks run (1 failed, 1 succeeded)
flake8
-
-
reviewboard/notifications/email/message.py (Diff revision 4) E128 continuation line under-indented for visual indent
-
reviewboard/notifications/email/message.py (Diff revision 4) E101 indentation contains mixed spaces and tabs
-
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 5 (+98 -72) |
Checks run (1 failed, 1 succeeded)
flake8
-
-
reviewboard/notifications/email/message.py (Diff revision 5) E101 indentation contains mixed spaces and tabs
-
reviewboard/notifications/email/message.py (Diff revision 5) E128 continuation line under-indented for visual indent
-
reviewboard/notifications/email/message.py (Diff revision 5) E101 indentation contains mixed spaces and tabs
-
reviewboard/notifications/email/message.py (Diff revision 5) E128 continuation line under-indented for visual indent
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 6 (+96 -72) |
Checks run (2 succeeded)
Testing Done: |
|
---|
-
-
reviewboard/notifications/webhooks.py (Diff revision 6) Indentation will need to be fixed here, since
logger
is one character shorter thanlogging
. Please check all your changed instances for this.
Change Summary:
fixing description and testing done text wrap around 80 chars
Commits: |
|
||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 7 (+96 -72) |
Checks run (2 succeeded)
Description: |
|
|||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
-
Just some trivial formatting-related things.
-
reviewboard/notifications/email/message.py (Diff revision 7) Please revert the changes to these lines.
-
reviewboard/notifications/webhooks.py (Diff revision 7) These lines need to be dedented one space, because "logger" is one character shorter than "logging".
-
-
-
-
-
-
Commits: |
|
||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Branch: |
|
||||||||||||||||
Diff: |
Revision 8 (+32 -34) |