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	ID	Author
Replace non-named logging with named logging for reviewboard.diffview 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.	275c6085f9a108507279031e5bdec98153dab828	Michelle
Fixed formatting issues Added two blank lines around the loggers instance declarations to denote that it is a separate section Removed extra spaces which were making attribute values in logging functions no longer line up with the other attributes	b20c33b0f96792e529278d19ae9d319599285015	Michelle

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:

Summary	ID	Author
Replace non-named logging with named logging for reviewboard.diffview 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.	275c6085f9a108507279031e5bdec98153dab828	Michelle
Replace non-named logging with named logging for reviewboard.diffview 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.	275c6085f9a108507279031e5bdec98153dab828	Michelle
Fixed formatting issues Added two blank lines around the loggers instance declarations to denote that it is a separate section Removed extra spaces which were making attribute values in logging functions no longer line up with the other attributes	b20c33b0f96792e529278d19ae9d319599285015	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)