-
-
-
-
-
Hopefully we never get files with \r\n, or everything above will fail. I don't know if we can, but it seems possible.
-
Thinking we might want to be more future-proof and structure this so we could add more later than a message and diff without just tacking onto tuples. We have a ChangeSet object in scmtools.core. I wonder if we can evolve that.
-
-
This is hard to read. Can we break them up by lines? I know we don't have to deal with the contents much, but it'd make debugging and reading comprehension so much nicer.
-
I was really unsure what was going on at first with 'testcase' and 'self'. Rather than have a testcase variable, can we instead call the inner 'self' something like 'service'? This is the pattern some other tests have.
-
-
-
-
Add a get_change method to SCMTool and HostingService
Review Request #4284 — Created July 3, 2013 and submitted
Add a get_change method to SCMTool and HostingService This change adds the final piece of integration API on the backend that we need for post-commit reviews--the ability to fetch the commit message and diff from the version control system given a revision ID. This is implemented for SVN and GitHub, just like the get_branches and get_commits methods.
- Ran unit tests. - Used this with my post-commit UI for both github and svn repos.
| Description | From | Last Updated |
|---|---|---|
|
Rather than building a big string, we should build an array of lines. It's more efficient. |
chipx86
|
|
|
No blank line. |
chipx86
|
|
|
No blank line. |
chipx86
|
|
|
Hopefully we never get files with \r\n, or everything above will fail. I don't know if we can, but it … |
chipx86
|
|
|
Thinking we might want to be more future-proof and structure this so we could add more later than a message … |
chipx86
|
|
|
I don't think we need this anymore. That was only important for Python 2.4. |
chipx86
|
|
|
This is hard to read. Can we break them up by lines? I know we don't have to deal with … |
chipx86
|
|
|
I was really unsure what was going on at first with 'testcase' and 'self'. Rather than have a testcase variable, … |
chipx86
|
|
|
No blank line. Same below. |
chipx86
|
|
|
""" on the next line. |
chipx86
|
|
|
""" on the next line. |
chipx86
|
|
|
Can you use prefix="reviewboard-svn."? This will make it easier to locate stale temp directories. |
chipx86
|
|
|
local variable 'testcase' is assigned to but never used |
reviewbot
|
|
|
Col: 32 E201 whitespace after '{' |
reviewbot
|
|
|
Col: 59 E202 whitespace before '}' |
reviewbot
|
|
|
Col: 35 E201 whitespace after '{' |
reviewbot
|
|
|
Col: 66 E202 whitespace before '}' |
reviewbot
|
|
|
Col: 29 E201 whitespace after '[' |
reviewbot
|
|
|
Col: 31 E201 whitespace after '{' |
reviewbot
|
|
|
Col: 49 E202 whitespace before '}' |
reviewbot
|
|
|
Col: 51 E202 whitespace before ']' |
reviewbot
|
|
|
Col: 30 E201 whitespace after '{' |
reviewbot
|
|
|
Col: 46 E202 whitespace before '}' |
reviewbot
|
|
|
Col: 1 W293 blank line contains whitespace |
reviewbot
|
|
|
Col: 1 W293 blank line contains whitespace |
reviewbot
|
|
|
Col: 1 W293 blank line contains whitespace |
reviewbot
|
|
|
Col: 1 W293 blank line contains whitespace |
reviewbot
|
|
|
Col: 9 E301 expected 1 blank line, found 0 |
reviewbot
|
|
|
I think this is actually worse. I was hoping something indented in the proper indentation level that just wraps sanely. … |
chipx86
|
|
|
Col: 1 W293 blank line contains whitespace |
reviewbot
|
|
|
Col: 1 W293 blank line contains whitespace |
reviewbot
|
|
|
Col: 1 W293 blank line contains whitespace |
reviewbot
|
|
|
Col: 1 W293 blank line contains whitespace |
reviewbot
|
|
|
Col: 80 E501 line too long (99 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (99 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (102 > 79 characters) |
reviewbot
|
-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: reviewboard/scmtools/models.py reviewboard/scmtools/tests.py reviewboard/scmtools/core.py reviewboard/hostingsvcs/service.py reviewboard/hostingsvcs/github.py reviewboard/hostingsvcs/tests.py reviewboard/scmtools/svn.py Ignored Files: -
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
This is a review from Review Bot. Tool: Pyflakes Processed Files: reviewboard/scmtools/models.py reviewboard/scmtools/tests.py reviewboard/scmtools/core.py reviewboard/hostingsvcs/service.py reviewboard/hostingsvcs/github.py reviewboard/hostingsvcs/tests.py reviewboard/scmtools/svn.py Ignored Files: -
-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: reviewboard/scmtools/models.py reviewboard/scmtools/tests.py reviewboard/scmtools/core.py reviewboard/hostingsvcs/service.py reviewboard/hostingsvcs/github.py reviewboard/hostingsvcs/tests.py reviewboard/scmtools/svn.py Ignored Files: -
-
-
-
-
This is a review from Review Bot. Tool: Pyflakes Processed Files: reviewboard/scmtools/models.py reviewboard/scmtools/tests.py reviewboard/scmtools/core.py reviewboard/hostingsvcs/service.py reviewboard/hostingsvcs/github.py reviewboard/hostingsvcs/tests.py reviewboard/scmtools/svn.py Ignored Files:
-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: reviewboard/scmtools/models.py reviewboard/scmtools/tests.py reviewboard/scmtools/core.py reviewboard/hostingsvcs/service.py reviewboard/hostingsvcs/github.py reviewboard/hostingsvcs/tests.py reviewboard/scmtools/svn.py Ignored Files: -
-
-
-
This is a review from Review Bot. Tool: Pyflakes Processed Files: reviewboard/scmtools/models.py reviewboard/scmtools/tests.py reviewboard/scmtools/core.py reviewboard/hostingsvcs/service.py reviewboard/hostingsvcs/github.py reviewboard/hostingsvcs/tests.py reviewboard/scmtools/svn.py Ignored Files: