maubin got a fish trophy!

Summary:

Replace non-named logging with named logging for reviewboard.diffviewer

Review Request #11811 — Created Sept. 17, 2021 and submitted March 24, 2022, 1:24 p.m.

Information
Owner:	maubin
Repository:	Review Board
Branch:	release-4.0.x
Bugs:	4903
Depends On:
Reviewers
Groups:	reviewboard
People:

Description

While reviewboard.diffviewer already uses some named logging, this commit
replaces all other usages of non-named logging with named logging instead.

Switching to named logging allows the logging mechanism to automatically
include information about the module and where the logging message came
from.

Testing Done

Ran unit tests for reviewboard.diffviewer and passed all tests.

Commits

Summary		Author
	Replace non-named logging with named logging for reviewboard.diffview	Michelle
	Fixed formatting issues	Michelle

Screenshots

Issues

Description	From	Last Updated
The summary, description, and testing all mention "diffview", but the name of the module is "diffviewer".	chipx86	Sept. 17, 2021, 2:17 p.m.
The description should wrap to around 75 characters, in order to ensure it fits properly in a Git diff. This …	chipx86	Sept. 17, 2021, 2:17 p.m.
Just realized, this seems to be missing the bug number. You can fill that in on the "Bugs" field on …	chipx86	Sept. 17, 2021, 2:18 p.m.
For the top-level of a module, we use two blank lines between sections. The constants above are one section, and …	chipx86	Sept. 17, 2021, 2:18 p.m.
The attribute values on the following two lines no longer line up with the one on the first line.	chipx86	Sept. 17, 2021, 2:18 p.m.
Same as in the other file, you'll want two blank lines on either side of this declaration.	chipx86	Sept. 17, 2021, 2:18 p.m.
Same as in the other file, the parameters no longer align. This repeats for the changes made below, so make …	chipx86	Sept. 17, 2021, 2:18 p.m.

flake8 passed.

JSHint passed.

Show all issues

The summary, description, and testing all mention "diffview", but the name of the module is "diffviewer".

Show all issues

The description should wrap to around 75 characters, in order to ensure it fits properly in a Git diff. This is where it helps to make sure the description is written when making a commit, as that is often configured to impose a line length limit.

reviewboard/diffviewer/diffutils.py (Diff revision 1)

Show all issues

For the top-level of a module, we use two blank lines between sections. The constants above are one section, and this logger can be seen as another section (instance declarations). Functions are another section.
Just like how there were two blank lines between _PATCH_GARBAGE_INPUT and def convert_to_unicode, you'll want to have two blank lines on either side of logger =.

reviewboard/diffviewer/diffutils.py (Diff revision 1)

Show all issues

The attribute values on the following two lines no longer line up with the one on the first line.

reviewboard/diffviewer/views.py (Diff revision 1)

Show all issues

Same as in the other file, you'll want two blank lines on either side of this declaration.

reviewboard/diffviewer/views.py (Diff revision 1)

Show all issues

Same as in the other file, the parameters no longer align.

This repeats for the changes made below, so make sure that those are fixed as well.

Show all issues

Just realized, this seems to be missing the bug number. You can fill that in on the "Bugs" field on the right.

Change Summary:

Fixed formatting issues in the code and review request description, added bug number

Summary:

-	Replace non-named logging with named logging for reviewboard.diffview
+	Replace non-named logging with named logging for reviewboard.diffviewer

Description:

~		While reviewboard.diffview already uses some named logging, this commit replaces all other usages of non-named logging with named logging instead.
~		Switching to named logging allows the logging mechanism to automatically include information about the module and where the logging message came from.
	~	While reviewboard.diffviewer already uses some named logging, this commit
	~	replaces all other usages of non-named logging with named logging instead.
	+
	+	Switching to named logging allows the logging mechanism to automatically
	+	include information about the module and where the logging message came
	+	from.

Testing Done:

~		Ran unit tests for reviewboard.diffview and passed all tests.
	~	Ran unit tests for reviewboard.diffviewer and passed all tests.

Commits:

		Author
-	Replace non-named logging with named logging for reviewboard.diffview	Michelle
+	Replace non-named logging with named logging for reviewboard.diffview	Michelle
+	Fixed formatting issues	Michelle

Bugs:

+	4903

Diff:

Revision 2 (+98 -86)

Show changes

	reviewboard/diffviewer/diffutils.py
	reviewboard/diffviewer/views.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to django-3.2 (0ded004)