flake8
passed.
JSHint
passed.
Updated Accounts module to use named logging.
Review Request #11810 — Created Sept. 17, 2021 and discarded
Only some logging statements in the Accounts module used named logging, when all of them should. So all logging statements were given the name "logger", and that was used instead.
Ex.
logging.warning("Some warning message")
becomes
logger = logging.getLogger(__name__) logger.warning("This is a warning to a named logger")
Python unit testing done.
| Summary | ID | Author |
|---|---|---|
| 1f333bfbcd10965808d380e82d67944758a11bb0 | mxiuwang |
| Description | From | Last Updated |
|---|---|---|
|
The description just repeats the summary. It helps to have a clear description of the purpose of the change. I … |
 chipx86
|
|
|
The failed tests are due to the setup issues with cffi and such. They're good for us to know, but … |
chipx86
|
|
|
Is there a bug number for this? If so, can you put it in the "Bugs" field? |
chipx86
|
|
|
The description needs to wrap at around 75 characters. This is important for ensuring that the text can properly fit … |
chipx86
|
|
|
In the top level of modules, we use 2 blank lines between sections. Imports are one section. Variable declarations are … |
chipx86
|
|
|
The parameters on the second line no longer align with the parameter on the first line, since logger is 1 … |
chipx86
|
|
|
This statement still has alignment errors. |
chipx86
|
|
|
There's an alignment error between these lines. |
chipx86
|
|
|
Since this module is now using named loggers, "X509Backend:" is no longer needed. That'll be covered by the logger name. … |
chipx86
|
|
|
This statement still has alignment issues. |
chipx86
|
|
|
This statement still has some alignment problems for the parameters. |
chipx86
|
-
-
The description just repeats the summary. It helps to have a clear description of the purpose of the change. I like to follow the problem/solution format, describing the problem before (log entries from this module didn't include the module name, so it was more difficult to know the context) and then describing the solution (named loggers).
-
The failed tests are due to the setup issues with
cffiand such. They're good for us to know, but not good history to keep around, since it's not related to the change. You can remove those. -
-
reviewboard/accounts/managers.py (Diff revision 1) In the top level of modules, we use 2 blank lines between sections. Imports are one section. Variable declarations are another. Classes are another.
Just as there were two blank lines between the imports and the class, you'll want to two blank lines before and after
logger = ....This will apply to the other modules as well.
-
reviewboard/accounts/managers.py (Diff revision 1) The parameters on the second line no longer align with the parameter on the first line, since
loggeris 1 character shorter thanlogging. Can you update these, and everywhere else that this change was made?
Change Summary:
Fixed comments
Description: |
|
||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
||||||||||||||||||||||||
Commits: |
|
||||||||||||||||||||||||
Bugs: |
|
||||||||||||||||||||||||
Diff: |
Revision 2 (+108 -66) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
-
-
The description needs to wrap at around 75 characters. This is important for ensuring that the text can properly fit in a Git commit.
-
reviewboard/accounts/backends/registry.py (Diff revision 2) This statement still has alignment errors.
-
reviewboard/accounts/backends/standard.py (Diff revision 2) There's an alignment error between these lines.
-
reviewboard/accounts/backends/x509.py (Diff revision 2) Since this module is now using named loggers, "X509Backend:" is no longer needed. That'll be covered by the logger name. Can you remove these prefixes?
-
-
reviewboard/accounts/views.py (Diff revision 2) This statement still has some alignment problems for the parameters.
