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.
-
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: |
|
|||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
-
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: |
|
|---|
Description: |
|
|---|
Tool: PEP8 Style Checker Processed Files: reviewboard/hostingsvcs/github.py Tool: Pyflakes Processed Files: reviewboard/hostingsvcs/github.py