Fixed incorrectly storing GitHub commit dates as tuples
Review Request #6874 — Created Feb. 1, 2015 and submitted
Remove one unnecessary comma in line 763 in reviewboard.Hostingsvcs.github. Line 763 is an assignment statement, which originally ends with a comma. This comma causes some problems:
- The variable is coerced into being a tuple istead of a str because of the comma, which is unexpected.
- It is confusing and against coding style.
The change avoids future problems caused by this inappropriate typecasting behaviour and keeps coding style consistent.
Passed original unit tests in
reviewboard.Hostingsvcs.tests.
- Groups:
-
The code looks fine, but the summary and description needs work.
I put up a guide over the weekend that I'm strongly encouraging everyone to read, which covers how to write good summaries/descriptions, with Dos and Do Nots: https://www.reviewboard.org/docs/codebase/dev/writing-good-descriptions/
One comment about your change: The comma is not just a code style issue. By having that comma in there, the result was being returned as a tuple and not as a string. That can actually introduce major probelms. The description should reflect that.
- Description:
-
~ Remove one unnecessary comma in line 763 in reviewboard.Hostingsvcs.github. Line 763 is an assignment statement, which ends with a comma. This comma in here is confusing and against coding style. Although it doesn't affect the functionality in that file.
~ Remove one unnecessary comma in line 763 in reviewboard.Hostingsvcs.github. Line 763 is an assignment statement, which originally ends with a comma. This comma causes some problems:
+ + - The type of variable
databecomestuplefromstrbecause of the comma, this typecasting behaviour might introduce major problems in the future.
+ - It is confusing and against coding style.
+ + The change avoids furture problems caused by this inappropriate typecasting behaviour and keeps coding style consistent.
- The type of variable
- Testing Done:
-
~ Unit tests passed.
~ Passed original unit tests in
reviewboard.Hostingsvcs.tests.
-
The code is a "Ship it" from me - but there's a typo in the Description - "furture".
Also,
The type of variable data becomes tuple from str because of the comma, this typecasting behaviour might introduce major problems in the future.can probably just be:
The variable is coerced into being a tuple istead of a str because of the comma, which is unexpected.
- Description:
-
Remove one unnecessary comma in line 763 in reviewboard.Hostingsvcs.github. Line 763 is an assignment statement, which originally ends with a comma. This comma causes some problems:
~ - The type of variable
databecomestuplefromstrbecause of the comma, this typecasting behaviour might introduce major problems in the future.
~ - The variable is coerced into being a tuple istead of a str because of the comma, which is unexpected.
- It is confusing and against coding style.
The change avoids furture problems caused by this inappropriate typecasting behaviour and keeps coding style consistent.
- The type of variable
- Description:
-
Remove one unnecessary comma in line 763 in reviewboard.Hostingsvcs.github. Line 763 is an assignment statement, which originally ends with a comma. This comma causes some problems:
- The variable is coerced into being a tuple istead of a str because of the comma, which is unexpected.
- It is confusing and against coding style.
~ The change avoids furture problems caused by this inappropriate typecasting behaviour and keeps coding style consistent.
~ The change avoids future problems caused by this inappropriate typecasting behaviour and keeps coding style consistent.
Tool: PEP8 Style Checker Processed Files: reviewboard/hostingsvcs/github.py Tool: Pyflakes Processed Files: reviewboard/hostingsvcs/github.py