code review 9626045: codereview.py: improve error message when user is not a...

codereview.py: improve error message when user is not a committer

[francesc, 2013-05-22 01:21:52]

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

I'd like you to review this change to
https://code.google.com/p/go

[bradfitz, 2013-05-22 01:28:39]

It should look for a 401 first before saying that



On Tue, May 21, 2013 at 6:21 PM, <campoy@golang.org> wrote:

> Reviewers: adg,
>
> Message:
> Hello adg@golang.org (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> codereview.py: improve error message when user is not a committer
>
> Please review this at
https://codereview.appspot.**com/9626045/<https://codereview.appspot.com/9626...
>
> Affected files:
>   M lib/codereview/codereview.py
>
>
> Index: lib/codereview/codereview.py
> ==============================**==============================**=======
> --- a/lib/codereview/codereview.py
> +++ b/lib/codereview/codereview.py
> @@ -1960,6 +1960,12 @@
>                                 # Remote repository had changes we missed.
>                                 need_sync()
>                         raise
> +               except urllib2.HTTPError, e:
> +                       print >>sys.stderr, "pushing to remote server
> failed. are you allowed to commit?"
> +                       if e.url.find("code.google.com") >= 0:
> +                               people = ui.expandpath("default")+"/**
> people/list"
> +                               print >>sys.stderr, "check you are a
> committer in ", people
> +                       raise
>         except:
>                 real_rollback()
>                 raise
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[francesc, 2013-05-22 01:35:34]

It actually returns 404, output of a test:


pushing to remote server failed. are you allowed to commit?
you should be in the people list:
https://code.google.com/p/gophorge/people/list
abort: HTTP Error 404: Not Found



On Tue, May 21, 2013 at 6:28 PM, Brad Fitzpatrick <bradfitz@golang.org>wrote:

> It should look for a 401 first before saying that
>
>
>
> On Tue, May 21, 2013 at 6:21 PM, <campoy@golang.org> wrote:
>
>> Reviewers: adg,
>>
>> Message:
>> Hello adg@golang.org (cc: golang-dev@googlegroups.com),
>>
>> I'd like you to review this change to
>> https://code.google.com/p/go
>>
>>
>> Description:
>> codereview.py: improve error message when user is not a committer
>>
>> Please review this at
https://codereview.appspot.**com/9626045/<https://codereview.appspot.com/9626...
>>
>> Affected files:
>>   M lib/codereview/codereview.py
>>
>>
>> Index: lib/codereview/codereview.py
>> ==============================**==============================**=======
>> --- a/lib/codereview/codereview.py
>> +++ b/lib/codereview/codereview.py
>> @@ -1960,6 +1960,12 @@
>>                                 # Remote repository had changes we missed.
>>                                 need_sync()
>>                         raise
>> +               except urllib2.HTTPError, e:
>> +                       print >>sys.stderr, "pushing to remote server
>> failed. are you allowed to commit?"
>> +                       if e.url.find("code.google.com") >= 0:
>> +                               people = ui.expandpath("default")+"/**
>> people/list"
>> +                               print >>sys.stderr, "check you are a
>> committer in ", people
>> +                       raise
>>         except:
>>                 real_rollback()
>>                 raise
>>
>>
>>
>> --
>>
>> ---You received this message because you are subscribed to the Google
>> Groups "golang-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
>> .
>> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
>> .
>>
>>
>>
>


-- 
--
Francesc

[bradfitz, 2013-05-22 03:04:05]

okay, I don't know their API.  I defer to adg.

[adg, 2013-05-22 04:28:02]

https://codereview.appspot.com/9626045/diff/2002/lib/codereview/codereview.py
File lib/codereview/codereview.py (right):

https://codereview.appspot.com/9626045/diff/2002/lib/codereview/codereview.py...
lib/codereview/codereview.py:1965: if e.url.find("code.google.com") >= 0:
this is a bit gratuitous. i think the previous message is enough

[francesc, 2013-05-22 04:38:45]

PTAL

https://codereview.appspot.com/9626045/diff/2002/lib/codereview/codereview.py
File lib/codereview/codereview.py (right):

https://codereview.appspot.com/9626045/diff/2002/lib/codereview/codereview.py...
lib/codereview/codereview.py:1965: if e.url.find("code.google.com") >= 0:
On 2013/05/22 04:28:02, adg wrote:
> this is a bit gratuitous. i think the previous message is enough

Done.

[r, 2013-05-22 04:54:10]

https://codereview.appspot.com/9626045/diff/8002/lib/codereview/codereview.py
File lib/codereview/codereview.py (right):

https://codereview.appspot.com/9626045/diff/8002/lib/codereview/codereview.py...
lib/codereview/codereview.py:1964: print >>sys.stderr, "pushing to remote server
failed. are you allowed to commit?"
change the punctuation so you don't need to start a sentence.

"pushing to remote server failed; do you have commit permissions?"

[francesc, 2013-05-22 22:27:42]

PTAL

https://codereview.appspot.com/9626045/diff/8002/lib/codereview/codereview.py
File lib/codereview/codereview.py (right):

https://codereview.appspot.com/9626045/diff/8002/lib/codereview/codereview.py...
lib/codereview/codereview.py:1964: print >>sys.stderr, "pushing to remote server
failed. are you allowed to commit?"
On 2013/05/22 04:54:10, r wrote:
> change the punctuation so you don't need to start a sentence.
> 
> "pushing to remote server failed; do you have commit permissions?"

Done.

[adg, 2013-05-22 23:10:30]

LGTM

[francesc, 2013-05-22 23:27:43]

*** Submitted as https://code.google.com/p/go/source/detail?r=cc83df684054 ***

codereview.py: improve error message when user is not a committer

R=adg, bradfitz, r
CC=golang-dev
https://codereview.appspot.com/9626045