code review 4524045: codereview: fetch metadata using JSON API, not XML scraping

codereview: fetch metadata using JSON API, not XML scraping

Fixes hg clpatch.

[rsc, 2011-05-12 02:56:37]

Hello golang-dev (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[rsc, 2011-05-12 02:57:07]

For once it got shorter!

[r2, 2011-05-12 03:02:33]

You do not have permission to view this issue

that's all i saw.

-rob

[rsc, 2011-05-12 03:15:21]

Hello golang-dev@googlegroups.com, r@google.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[r, 2011-05-12 03:19:45]

LGTM

http://codereview.appspot.com/4524045/diff/4001/lib/codereview/codereview.py
File lib/codereview/codereview.py (right):

http://codereview.appspot.com/4524045/diff/4001/lib/codereview/codereview.py#...
lib/codereview/codereview.py:49: # json is new in 2.6.
2.6 what?
also JSON should be capitalized.

http://codereview.appspot.com/4524045/diff/4001/lib/codereview/codereview.py#...
lib/codereview/codereview.py:50: # before that it was an external package named
simplejson.
s/b/B/

http://codereview.appspot.com/4524045/diff/4001/lib/codereview/codereview.py#...
lib/codereview/codereview.py:64: raise e
aha. now i understand someone's question about go imports.
still, is this really what you need to do? this feels like something that
belongs in a makefile.

http://codereview.appspot.com/4524045/diff/4001/lib/codereview/codereview.py#...
lib/codereview/codereview.py:538: cl.private = d.get('private', 'false') !=
'false'
pardon my ignorance but isn't !=false ==true?

http://codereview.appspot.com/4524045/diff/4001/lib/codereview/codereview.py#...
lib/codereview/codereview.py:2026: d = coerce_to_utf8(d)
d = coerce_t_utf8(json.loads(data)) ?

http://codereview.appspot.com/4524045/diff/4001/lib/codereview/codereview.py#...
lib/codereview/codereview.py:2045: return x
gawd this is awful.

[rsc, 2011-05-12 03:26:15]

>
http://codereview.appspot.com/4524045/diff/4001/lib/codereview/codereview.py#...
> lib/codereview/codereview.py:49: # json is new in 2.6.
> 2.6 what?
> also JSON should be capitalized.

Done.

# The standard 'json' package is new in Python 2.6.
# Before that it was an external package named simplejson.

>
http://codereview.appspot.com/4524045/diff/4001/lib/codereview/codereview.py#...
> lib/codereview/codereview.py:64: raise e
> aha. now i understand someone's question about go imports.
> still, is this really what you need to do? this feels like something
> that belongs in a makefile.

Sadly, it is what you need to do, at least as long as
we have people running old Pythons.

>
http://codereview.appspot.com/4524045/diff/4001/lib/codereview/codereview.py#...
> lib/codereview/codereview.py:538: cl.private = d.get('private', 'false')
> != 'false'
> pardon my ignorance but isn't !=false ==true?

Not if the value == 3.14 to begin with.  :-)

>
http://codereview.appspot.com/4524045/diff/4001/lib/codereview/codereview.py#...
> lib/codereview/codereview.py:2026: d = coerce_to_utf8(d)
> d = coerce_t_utf8(json.loads(data)) ?

Done.

>
http://codereview.appspot.com/4524045/diff/4001/lib/codereview/codereview.py#...
> lib/codereview/codereview.py:2045: return x
> gawd this is awful.

You probably didn't see the final addition:

	if type(x) is str:
		x = x.replace('\r\n', '\n')

Russ

[rsc, 2011-05-12 03:26:58]

*** Submitted as http://code.google.com/p/go/source/detail?r=5fb62921a952 ***

codereview: fetch metadata using JSON API, not XML scraping

Fixes hg clpatch.

R=golang-dev, r, r
CC=golang-dev
http://codereview.appspot.com/4524045