Summary

Switch to using named logging inside cmdline folder

Review Request #11954 — Created Jan. 21, 2022 and submitted March 24, 2022, 12:09 p.m.

Information

Owner

gdehal

Repository

Review Board

Branch

release-4.0.x

Bugs

4902

Depends On

Reviewers

Groups

reviewboard

People

Description

Instantiate a logger and use that instead of calling logging function in cmdline folder

Testing Done

Tested by calling the scripts in the command line and verified that the error logs were being printed when using logger.error instead of logging.error.

Commits

Summary	ID
Switch to using named logging inside cmdline folder	e70345053ebd260cf298b3de1ea00753e7df907f

Issues

Description	From	Last Updated
Please add the bug number to the "Bugs" field.	david	Jan. 22, 2022, 10:22 a.m.
Indentation on these lines needs to be changed to match (logger is one character shorter than logging).	david	Jan. 22, 2022, 10:22 a.m.
Can you revert the change to this file?	david	Jan. 22, 2022, 10:25 a.m.
Python wants two blank lines around top-level items in a file.	david	Jan. 22, 2022, 10:25 a.m.
Indentation on this line needs to be changed to match (logger is one character shorter than logging). Let's also put …	david	Jan. 22, 2022, 10:25 a.m.
Indentation on this line needs to be changed to match (logger is one character shorter than logging). Let's also put …	david	Jan. 22, 2022, 10:25 a.m.
W291 trailing whitespace	reviewbot	Jan. 22, 2022, 10:31 a.m.
W291 trailing whitespace	reviewbot	Jan. 22, 2022, 10:31 a.m.

flake8 passed.

JSHint passed.

Description:

~		Switch from using logging to logger
	~	Switch from using logging to logger in cmdline folder

Hi Gurvir! So there's a bunch of things in here that don't seem to be related to your patch... did you make sure to create this commit on top of a branch off of 4.0.x?

Also, we need a bit more for the summary / description / testing done. Please see https://www.notion.so/reviewboard/Writing-Good-Change-Descriptions-10529e7c207743fa8ca90153d4b21fea

Commits:

	Summary	ID
	Switched from using logging to logger	ff24bc18bd6fe88c5a7f47c9ad84a9ccff6caa9c
	Switched from using logging to logger	af1a352f0098b7faf6b0b8fd30299695dcd028f0

Diff:

Revision 2 (+24 -12)

Show changes

	reviewboard/cmdline/rbext.py
	reviewboard/cmdline/rbsite.py
	reviewboard/cmdline/rbssh.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Here's an example of a review request doing one of these named logger changes with a more fleshed out description / testing done: https://reviews.reviewboard.org/r/11811/

Description:

~		Switch from using logging to logger in cmdline folder
	~	Instantiate a logger and use that instead of calling logging function in cmdline folder

Testing Done:

Tested by calling the scripts in the command line and verified that the error logs were being printed when using logger.error instead of logging.error.

Commits:

	Summary	ID
	Switched from using logging to logger	af1a352f0098b7faf6b0b8fd30299695dcd028f0
	Switched from using logging to logger	af1a352f0098b7faf6b0b8fd30299695dcd028f0
	Change logger.basicConfig to use logging	9f73de7300aeb00483f260ea8591448bd07d627d

Diff:

Revision 3 (+22 -16)

Show changes

	reviewboard/cmdline/rbext.py
	reviewboard/cmdline/rbsite.py
	reviewboard/cmdline/rbssh.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Summary:

-	Switched from using logging to logger
+	Switch to using named logging inside cmdline folder

The issue has been resolved. Show all issues

Please add the bug number to the "Bugs" field.

reviewboard/cmdline/rbext.py (Diff revision 3)

The issue has been resolved. Show all issues

Indentation on these lines needs to be changed to match (logger is one character shorter than logging).

reviewboard/cmdline/rbsite.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Can you revert the change to this file?
```
reviewboard/cmdline/rbssh.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Python wants two blank lines around top-level items in a file.
```

reviewboard/cmdline/rbssh.py (Diff revision 3)

The issue has been resolved. Show all issues

Indentation on this line needs to be changed to match (logger is one character shorter than logging).

Let's also put the % on the second line rather than the first:

logger.error('...'
             % username)

reviewboard/cmdline/rbssh.py (Diff revision 3)

The issue has been resolved. Show all issues

Indentation on this line needs to be changed to match (logger is one character shorter than logging).

Let's also put the % on the second line rather than the first.

Change Summary:

Addressed code review comments

Commits:

	Summary	ID
	Switched from using logging to logger	af1a352f0098b7faf6b0b8fd30299695dcd028f0
	Change logger.basicConfig to use logging	9f73de7300aeb00483f260ea8591448bd07d627d
	Switch to using named logging inside cmdline folder	f7c285df25d4e4917d11241e7385638a76ef3913

Bugs:

+	4902

Diff:

Revision 4 (+32 -16)

Show changes

	reviewboard/cmdline/rbext.py
	reviewboard/cmdline/rbssh.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/cmdline/rbssh.py (Diff revision 4)
The issue has been dropped. Show all issues
```
W291 trailing whitespace
```
reviewboard/cmdline/rbssh.py (Diff revision 4)
The issue has been dropped. Show all issues
```
W291 trailing whitespace
```

Commits:

	Summary	ID
	Switch to using named logging inside cmdline folder	f7c285df25d4e4917d11241e7385638a76ef3913
	Switch to using named logging inside cmdline folder	e70345053ebd260cf298b3de1ea00753e7df907f

Diff:

Revision 5 (+32 -16)

Show changes

	reviewboard/cmdline/rbext.py
	reviewboard/cmdline/rbssh.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Status: Closed (submitted)

Change Summary:

Pushed to django-3.2 (0c0e034)

Ship it!

```
Ship It!
```