Wrong base revision for Python repository. Fixes issue 239.

[techtonik, 2010-10-12 23:49:53]

this should fix problems Martin is experiencing with ranged python.org reviews

http://code.google.com/p/rietveld/issues/detail?id=239

[techtonik, 2010-10-12 23:49:53]

this should fix problems Martin is experiencing with ranged python.org reviews

http://code.google.com/p/rietveld/issues/detail?id=239

[GvR, 2010-10-13 03:14:24]

I can't tell what part of your change is just a well-intentioned cleanup and
what part is the actual bugfix.

http://codereview.appspot.com/2442042/diff/7001/upload.py
File upload.py (left):

http://codereview.appspot.com/2442042/diff/7001/upload.py#oldcode912
upload.py:912: return None
Please don't delete this line. It is there for the (human) reader to know that
this function is expected to return a value and that the value returned here is
None.

http://codereview.appspot.com/2442042/diff/7001/upload.py
File upload.py (right):

http://codereview.appspot.com/2442042/diff/7001/upload.py#newcode879
upload.py:879: if line.startswith('URL'):
Is this a random cleanup or is there a reason you changed this test?

http://codereview.appspot.com/2442042/diff/7001/upload.py#newcode880
upload.py:880: url = line.split()[1]
This is worse than the original code -- if there was a line containing just the
string "URL" (or "URL" followed by junk not containing whitespace) this would
raise IndexError.

http://codereview.appspot.com/2442042/diff/7001/upload.py#newcode890
upload.py:890: elif netloc.endswith(".googlecode.com"):
Why drop support for Collabnet?

[techtonik, 2010-10-13 07:47:40]

To distinguish fixes and cleanups review patchsets one by one.
1. Fix
2,3. Cleanups
4. Review changes

http://codereview.appspot.com/2442042/diff/7001/upload.py
File upload.py (left):

http://codereview.appspot.com/2442042/diff/7001/upload.py#oldcode912
upload.py:912: return None
On 2010/10/13 03:14:24, GvR wrote:
> Please don't delete this line. It is there for the (human) reader to know that
> this function is expected to return a value and that the value returned here
is
> None.

Done.

http://codereview.appspot.com/2442042/diff/7001/upload.py
File upload.py (right):

http://codereview.appspot.com/2442042/diff/7001/upload.py#newcode879
upload.py:879: if line.startswith('URL'):
On 2010/10/13 03:14:24, GvR wrote:
> Is this a random cleanup or is there a reason you changed this test?

line.startswith("URL") is just more natural/readable.

http://codereview.appspot.com/2442042/diff/7001/upload.py#newcode880
upload.py:880: url = line.split()[1]
On 2010/10/13 03:14:24, GvR wrote:
> This is worse than the original code -- if there was a line containing just
the
> string "URL" (or "URL" followed by junk not containing whitespace) this would
> raise IndexError.

Changed to look for the lines that start with "URL: " instead. For 100%
protection it is better to parse "svn info --xml", but I haven't found any XPath
API in standard library to keep it simple.

http://codereview.appspot.com/2442042/diff/7001/upload.py#newcode890
upload.py:890: elif netloc.endswith(".googlecode.com"):
On 2010/10/13 03:14:24, GvR wrote:
> Why drop support for Collabnet?

Because SVN development is moved to Apache - see http://svn.collab.net/

[GvR, 2010-10-13 19:07:35]

LG. Martin confirmed that it works.

Can you test whether the default base-finding code for the apache svn repo
works, and if not, file a bug to remind us to fix that?

[techtonik, 2010-10-19 18:56:57]

Default base-finding code for the apache svn repo
works, so I close this.