Summary

Delete an unused variable in UsernameColumn.py

Review Request #9495 — Created Jan. 19, 2018 and submitted Feb. 12, 2018, 11:32 a.m.

Information

Owner

ridum

Repository

Review Board

Branch

release-3.0.x

Bugs

4630

Depends On

Commit

3ff35c2...

Reviewers

Groups

reviewboard, students

People

Description

The user_url variable is dead code that accidentally wasn't removed
in 43957396.
This patch removes the unused variable.

Testing Done

Manual: * Linted user_url and no longer saw an unused variable.

Automated: * Ran unit tests.

Issues

Description	From	Last Updated
Your summary should be a high-level overview of your change. It shouldn't require looking up the bug number.	brennie	Jan. 19, 2018, 12:13 p.m.
4630 should go in the bugs field.	brennie	Jan. 19, 2018, 12:13 p.m.
You'll need to fill out the testing done field.	brennie	Jan. 19, 2018, 12:15 p.m.
This change looks fine, but I have one minor nit regarding the Description: due to the recent update If you're …	mike_conley	Jan. 20, 2018, 7:50 a.m.
You can mark things as code using backquotes, e.g: The `user_url`	brennie	Jan. 21, 2018, 6:40 a.m.
Mind wrapping your description at 72chars?	brennie	Jan. 21, 2018, 6:41 a.m.
Mind formatting the testing done as: * Ran unit tests. * Linted the file and no longer saw an unused …	brennie	Jan. 21, 2018, 6:41 a.m.

flake8 passed.

JSHint passed.

You will want to give our guide on writing on change descriptions a read.

The issue has been resolved. Show all issues

Your summary should be a high-level overview of your change. It shouldn't require looking up the bug number.

The issue has been resolved. Show all issues
```
4630 should go in the bugs field.
```

The issue has been resolved. Show all issues

You'll need to fill out the testing done field.

Summary:: fix 4630
delete unused varibale in UsernameColumn.py
Testing Done:: +
Manual:
+ Using find to global search the dependency of "user_url"
+ Automated:
+ running runtest.py
Bugs:: 4630

The issue has been resolved. Show all issues

This change looks fine, but I have one minor nit regarding the Description:

due to the recent update

If you're referring to a particular update, you might want to mention which one.

I just took a quick look at reviewboard/datagrids/columns.py, and it looks like it was this change:

https://github.com/reviewboard/reviewboard/commit/439573964de820cb8663ba95db50ad02d9226794#diff-2a6f9481a47b04e51de14d94fc1999d1

So perhaps you could change this to:

The "user_url" variable is dead code that accidentally wasn't removed in 43957396. This patch removes the unused variable.

Description:: ~
"user_url" assignment in UsernameColumn.render_data is unused due to the recent update. Removed it from the code
~
The "user_url" variable is dead code that accidentally wasn't removed in 43957396. This patch removes the unused variable.

A few last small requests about the summary/description:

Please start your summary with a capital letter ("Delete ...")
You have a typo in the summary (varibale -> variable)
The summary should be "Delete an unused..."
Please wrap the description to 70 columns.

RI

ridum Jan. 20, 2018, 8:55 a.m.
```
done :P thanks for pointing out
```

This looks good to me. Thanks!

mike_conley Jan. 20, 2018, 8:43 a.m.

Whoops - revoking ship-it; I didn't notice that David had some follow-ups.

Summary:: delete unused varibale in UsernameColumn.py
Delete an unused variable in UsernameColumn.py
Description:: ~
The "user_url" variable is dead code that accidentally wasn't removed in 43957396. This patch removes the unused variable.
~
The "user_url" variable is dead code that accidentally
+ wasn't removed in 43957396.
+ This patch removes the unused variable.

The issue has been resolved. Show all issues

You can mark things as code using backquotes, e.g:

The `user_url`

The issue has been resolved. Show all issues
```
Mind wrapping your description at 72chars?
```

The issue has been resolved. Show all issues

Mind formatting the testing done as:

* Ran unit tests.
* Linted the file and no longer saw an unused variable.

Description:: ~
The "user_url" variable is dead code that accidentally
~ wasn't removed in 43957396.
~
The user_url variable is dead code that accidentally wasn't removed
~ in 43957396.
This patch removes the unused variable.
Testing Done:: ~
Manual:
~ Using find to global search the dependency of "user_url"
~ Automated:
~ running runtest.py
~
Manual:
~ * Linted user_url and no longer saw an unused variable.
~ Automated:
~ * Ran unit tests.

Testing Done:: Manual:
~ * Linted user_url and no longer saw an unused variable.
~ Automated:
~ * Linted user_url and no longer saw an unused variable.
~
+
Automated:
* Ran unit tests.

Ship it!

```
Looks good to me! Thanks!
```

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-3.0.x (8a8fd3e)

	+	Manual:
	+	Using find to global search the dependency of "user_url"
	+	Automated:
	+	running runtest.py

~		"user_url" assignment in UsernameColumn.render_data is unused due to the recent update. Removed it from the code
	~	The "user_url" variable is dead code that accidentally wasn't removed in 43957396. This patch removes the unused variable.

~		The "user_url" variable is dead code that accidentally wasn't removed in 43957396. This patch removes the unused variable.
	~	The "user_url" variable is dead code that accidentally
	+	wasn't removed in 43957396.
	+	This patch removes the unused variable.

		Manual:
~		* Linted `user_url` and no longer saw an unused variable.
~		Automated:
	~	* Linted `user_url` and no longer saw an unused variable.
	~
	+	Automated:
		* Ran unit tests.