Replacing non-named usages to use names logging in files in reviewboard.notifications

Review Request #11396 — Created Jan. 22, 2021 and submitted

Information

Review Board
master

Reviewers

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 ID Author
fixing indent errors
3d387ef246879fc77562bd1e538f4562cb16f2d5 anahita-m
fixing formatting issue with imports
fa90587358957685d770fc673b75f939e8216a5c anahita-m
Description From Last Updated

Can you wrap your description and testing done to be 80 characters in width?

daviddavid

If you are only importing logger from reviewboard.notifications.webhooks shouldn't you import it once at the top of the file?

ryankangryankang

Please put that second blank line back.

daviddavid

Each of these blank lines should be two.

daviddavid

Please put this blank line back.

daviddavid

We don't need a blank line here.

daviddavid

This blank line can go away.

daviddavid

Please add another blank line here.

daviddavid

E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

W191 indentation contains tabs

reviewbotreviewbot

E128 continuation line under-indented for visual indent

reviewbotreviewbot

E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

E128 continuation line under-indented for visual indent

reviewbotreviewbot

E231 missing whitespace after ','

reviewbotreviewbot

E128 continuation line under-indented for visual indent

reviewbotreviewbot

E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

W191 indentation contains tabs

reviewbotreviewbot

W191 indentation contains tabs

reviewbotreviewbot

E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

E128 continuation line under-indented for visual indent

reviewbotreviewbot

E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

E128 continuation line under-indented for visual indent

reviewbotreviewbot

Indentation will need to be fixed here, since logger is one character shorter than logging. Please check all your changed …

daviddavid

Please revert the changes to these lines.

daviddavid

These lines need to be dedented one space, because "logger" is one character shorter than "logging".

daviddavid

Dedent one space.

daviddavid

Dedent one space.

daviddavid

Dedent one space.

daviddavid

Dedent one space.

daviddavid

Dedent one space.

daviddavid

Dedent one space.

daviddavid
amohapatra
amohapatra
amohapatra
ryankang
  1. 
      
  2. If you are only importing logger from reviewboard.notifications.webhooks shouldn't you import it once at the top of the file?

    1. Yeah, I think you're right. I didn't notice that all of the methods being tested were coming from the same file and same logger. I just updated with new changes where I removed the localized import logger statement from those methods and moved the import to the top of the file instead.

  3. 
      
amohapatra
david
  1. 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.

  2. Can you wrap your description and testing done to be 80 characters in width?

    1. These still need to be rewrapped (maybe you fixed it in your local commit?)

    2. I think I was able to fix it finally, could you let me know if I did it correctly this time before I close the issue?

  3. reviewboard/notifications/email/message.py (Diff revision 2)
     
     
     
     
     

    Please put that second blank line back.

  4. reviewboard/notifications/email/utils.py (Diff revision 2)
     
     
     
     
     
     

    Each of these blank lines should be two.

  5. Please put this blank line back.

  6. We don't need a blank line here.

  7. This blank line can go away.

  8. reviewboard/notifications/webhooks.py (Diff revision 2)
     
     
     
     

    Please add another blank line here.

  9. 
      
amohapatra
Review request changed

Commits:

Summary ID Author
replacing non-named usages to use names logging in files in reviewboard.notifications
bdab39b26ba2a4e812aade7404ad2cbe7c886e1d anahita-m
replacing non-named usages to use names logging in files in reviewboard.notifications
a0fabcec3e461847f60067342e03d607bd955993 anahita-m

Diff:

Revision 3 (+98 -72)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

amohapatra
Review request changed

Commits:

Summary ID Author
replacing non-named usages to use names logging in files in reviewboard.notifications
a0fabcec3e461847f60067342e03d607bd955993 anahita-m
replacing non-named usages to use names logging in files in reviewboard.notifications
20dd6e8ed1657cad1731740b7b2b2b43b3d9414f anahita-m

Diff:

Revision 4 (+96 -72)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

amohapatra
Review request changed

Commits:

Summary ID Author
replacing non-named usages to use names logging in files in reviewboard.notifications
20dd6e8ed1657cad1731740b7b2b2b43b3d9414f anahita-m
replacing non-named usages to use names logging in files in reviewboard.notifications
812da5d3aa499bf29c87297c8172a80add2c1617 anahita-m

Diff:

Revision 5 (+98 -72)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

amohapatra
amohapatra
amohapatra
david
  1. 
      
  2. reviewboard/notifications/webhooks.py (Diff revision 6)
     
     
     
     
     

    Indentation will need to be fixed here, since logger is one character shorter than logging. Please check all your changed instances for this.

    1. I'm probably not understanding this correctly as I wasn't able to find any errors with indentation when I went through all the changed instances. I also ran flake8 on the changed files and did not see any errors regarding indentation.

  3. 
      
amohapatra
amohapatra
david
  1. Just some trivial formatting-related things.

  2. reviewboard/notifications/email/message.py (Diff revision 7)
     
     
     
     
     
     

    Please revert the changes to these lines.

  3. reviewboard/notifications/webhooks.py (Diff revision 7)
     
     
     
     

    These lines need to be dedented one space, because "logger" is one character shorter than "logging".

  4. reviewboard/notifications/webhooks.py (Diff revision 7)
     
     
     

    Dedent one space.

  5. reviewboard/notifications/webhooks.py (Diff revision 7)
     
     

    Dedent one space.

  6. reviewboard/notifications/webhooks.py (Diff revision 7)
     
     

    Dedent one space.

  7. reviewboard/notifications/webhooks.py (Diff revision 7)
     
     

    Dedent one space.

  8. reviewboard/notifications/webhooks.py (Diff revision 7)
     
     

    Dedent one space.

  9. reviewboard/notifications/webhooks.py (Diff revision 7)
     
     
     

    Dedent one space.

  10. 
      
amohapatra
david
  1. Going to make some fixes and get this landed. Thanks!

  2. 
      
amohapatra
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (f1ff734)
Loading...