Added logger global variable to have named logging in files in reviewboard.attachments
Review Request #11394 — Created Jan. 22, 2021 and discarded
Previously logging in
reviewboard/reviewboard/attachmentswas
un-named logging. This means it's hard to determine where the
error came from in the logs.To fix this a global
loggervariable was defined in each file
that adds the location of the file to the error message.
Added unit test for
unregister_mimetype_handlerthat passes it an
unregistered handler. This should cause an error to be logged. The
unit test listens to whether or not the logger ran.Ran
./tests/runtests.py reviewboard.attachmentsand passed all 32 tests.
| Summary | ID |
|---|---|
| af955d6577f0c7a5c14f95c82ad42a3a0810ab58 | |
| aa0b092d17a7b02fcc31d5df99c84226da95bdda | |
| b7f46628bd891c9c1150c845d3d68ef9562ae13b | |
| 53b24fff73becb17cb2211ad1f8f415a6157c4ca | |
| c2192c199be7af0d54bb2a58c635a7861ee45238 | |
| f4d9c978375b910815ea90fff71bfbd19ce91635 |
| Description | From | Last Updated |
|---|---|---|
|
Please wrap your description and testing done to 80 columns. That last sentence in testing done also needs a period … |
 david
|
|
|
Please add the bug number in the "bugs" field. |
david
|
|
|
Please capitalize the sentence in your summary. You can also refer to "reviewboard.attachments" instead of giving the file path. |
david
|
|
|
The diff as posted has a lot of extra stuff. It looks like you pulled on release-4.0.x but then didn't … |
david
|
|
|
E302 expected 2 blank lines, found 1 |
 reviewbot
|
|
|
E302 expected 2 blank lines, found 1 |
reviewbot
|
|
|
E501 line too long (111 > 79 characters) |
reviewbot
|
|
|
You'll need to update the indentation of the line with the arguments to match (since "logger" is one fewer character … |
david
|
|
|
Let's wrap this as such: """Testing if logger in unregister_mimetype_handler is called when an unregistered handler is passed in """ … |
david
|
|
|
Let's add another two blank lines above this. |
david
|
|
|
This should have two blank lines above and below it. |
david
|
- Description:
-
~ Changed logging in
reviewboard/reviewboard/attachments/mimetypes.pyandreviewboard/reviewboard/attachments/models.pyto named logging~ Changed logging in
reviewboard/reviewboard/attachments/mimetypes.pyandreviewboard/reviewboard/attachments/models.pyto named logging+ Added new test in MimetypeHandlerTeststhat tests if an error log is called whenunregister_mimetype_handlergets a handler that was never registered - Testing Done:
-
~ Added a test to
MimetypeHandlerTeststhat ensures an error is logged if a handler that is already unregistered is passed tounregister_mimetype_handler~ Ran
./tests/runtests.py reviewboard.attachmentsand passed all 32 tests - Commits:
-
Summary ID af955d6577f0c7a5c14f95c82ad42a3a0810ab58 aa0b092d17a7b02fcc31d5df99c84226da95bdda af955d6577f0c7a5c14f95c82ad42a3a0810ab58 aa0b092d17a7b02fcc31d5df99c84226da95bdda b7f46628bd891c9c1150c845d3d68ef9562ae13b
Checks run (2 succeeded)
- Groups:
- Description:
-
~ Changed logging in
reviewboard/reviewboard/attachments/mimetypes.pyandreviewboard/reviewboard/attachments/models.pyto named logging~ Added new test in MimetypeHandlerTeststhat tests if an error log is called whenunregister_mimetype_handlergets a handler that was never registered~ Previously logging in
reviewboard/reviewboard/attachmentswas un-named logging. This means it's hard to determine where the error came from in the logs.~ + To fix this a global
loggervariable was defined in each file that adds the location of the file to the error message. - Testing Done:
-
+ Added unit test for
unregister_mimetype_handlerthat passes it an unregistered handler. This should cause an error to be logged. The unit test listens to whether or not the logger ran.+ Ran
./tests/runtests.py reviewboard.attachmentsand passed all 32 tests
-
-
Please wrap your description and testing done to 80 columns. That last sentence in testing done also needs a period at the end.
-
-
Please capitalize the sentence in your summary. You can also refer to "reviewboard.attachments" instead of giving the file path.
-
You'll need to update the indentation of the line with the arguments to match (since "logger" is one fewer character than "logging"). Here and all the other places you changed.
-
Let's wrap this as such:
"""Testing if logger in unregister_mimetype_handler is called when an unregistered handler is passed in """Note also that I changed the spaces to underscores in that message to match the method name.
- Summary:
-
added logger global variable to have named logging in files in reviewboard/reviewboard/attachments/Added logger global variable to have named logging in files in reviewboard.attachments
- Description:
-
~ Previously logging in
reviewboard/reviewboard/attachmentswas un-named logging. This means it's hard to determine where the error came from in the logs.~ Previously logging in
reviewboard/reviewboard/attachmentswas+ un-named logging. This means it's hard to determine where the + error came from in the logs. ~ To fix this a global
loggervariable was defined in each file that adds the location of the file to the error message.~ To fix this a global
loggervariable was defined in each file+ that adds the location of the file to the error message. - Testing Done:
-
~ Added unit test for
unregister_mimetype_handlerthat passes it an unregistered handler. This should cause an error to be logged. The unit test listens to whether or not the logger ran.~ Added unit test for
unregister_mimetype_handlerthat passes it an+ unregistered handler. This should cause an error to be logged. The + unit test listens to whether or not the logger ran. ~ Ran
./tests/runtests.py reviewboard.attachmentsand passed all 32 tests~ Ran
./tests/runtests.py reviewboard.attachmentsand passed all 32 tests. - Commits:
-
Summary ID af955d6577f0c7a5c14f95c82ad42a3a0810ab58 aa0b092d17a7b02fcc31d5df99c84226da95bdda b7f46628bd891c9c1150c845d3d68ef9562ae13b af955d6577f0c7a5c14f95c82ad42a3a0810ab58 aa0b092d17a7b02fcc31d5df99c84226da95bdda b7f46628bd891c9c1150c845d3d68ef9562ae13b 53b24fff73becb17cb2211ad1f8f415a6157c4ca c2192c199be7af0d54bb2a58c635a7861ee45238 - Bugs:
-
- Diff:
Revision 3 (+94 -52)
Checks run (2 succeeded)
flake8 passed.JSHint passed.
- Change Summary:
-
added 2 blank spaces before and after all definitions of logger
- Commits:
-
Summary ID af955d6577f0c7a5c14f95c82ad42a3a0810ab58 aa0b092d17a7b02fcc31d5df99c84226da95bdda b7f46628bd891c9c1150c845d3d68ef9562ae13b 53b24fff73becb17cb2211ad1f8f415a6157c4ca c2192c199be7af0d54bb2a58c635a7861ee45238 af955d6577f0c7a5c14f95c82ad42a3a0810ab58 aa0b092d17a7b02fcc31d5df99c84226da95bdda b7f46628bd891c9c1150c845d3d68ef9562ae13b 53b24fff73becb17cb2211ad1f8f415a6157c4ca c2192c199be7af0d54bb2a58c635a7861ee45238 f4d9c978375b910815ea90fff71bfbd19ce91635 - Diff:
-
Revision 4 (+1702 -4519)