Use TRYBOT_RESULTS_EMAIL_ADDRESS environment variable for the try servers....

Use  TRYBOT_RESULTS_EMAIL_ADDRESS environment variable
for the try servers and fallback on EMAIL_ADDRESS, since
WebKit uses that.

The reason is that some people have WebKit emails that
predate Chromium emails so some separation is warranted
so that people don't have to fix up emails all the time.

BUG=None
TEST=None

[Mark Mentovai, 2009-11-08 05:02:17]

http://codereview.chromium.org/372048/diff/1/2
File trychange.py (right):

http://codereview.chromium.org/372048/diff/1/2#newcode456
Line 456: default=os.environ.get('CHROMIUM_EMAIL_ADDRESS'),
Do this:

default=os.environ.get('CHROMIUM_EMAIL_ADDRESS',
os.environ.get('EMAIL_ADDRESS)),

meaning that if CHROMIUM_EMAIL_ADDRESS is unset, it will fall back to
EMAIL_ADDRESS.

http://codereview.chromium.org/372048/diff/1/2#newcode596
Line 596: 'something like that.'
Space after period.

[Finnur, 2009-11-08 06:17:44]

Thanks for the comments.

However, the code sample you specified didn't work, but I have changed it so
that it falls back on EMAIL_ADDRESS if the other one isn't specified.

I also addressed the 'space after period' comment, but I changed it so that the
email address appears on the next line, so that we always respect the 80 cols
output in the console window and don't have weird line wrappings.

Uploaded new patch.

On 2009/11/08 05:02:17, Mark Mentovai wrote:
> http://codereview.chromium.org/372048/diff/1/2
> File trychange.py (right):
> 
> http://codereview.chromium.org/372048/diff/1/2#newcode456
> Line 456: default=os.environ.get('CHROMIUM_EMAIL_ADDRESS'),
> Do this:
> 
> default=os.environ.get('CHROMIUM_EMAIL_ADDRESS',
> os.environ.get('EMAIL_ADDRESS)),
> 
> meaning that if CHROMIUM_EMAIL_ADDRESS is unset, it will fall back to
> EMAIL_ADDRESS.
> 
> http://codereview.chromium.org/372048/diff/1/2#newcode596
> Line 596: 'something like that.'
> Space after period.

[M-A Ruel, 2009-11-08 18:23:59]

http://codereview.chromium.org/372048/diff/5/2001
File trychange.py (right):

http://codereview.chromium.org/372048/diff/5/2001#newcode455
Line 455: email = os.environ.get('CHROMIUM_EMAIL_ADDRESS');
Please use TRY_EMAIL_ADDRESS or TRY_JOB_EMAIL_ADDRESS (no preference). I try to
keep this script as generic as possible.

And I don't understand why you said this wouldn't work:

email = os.environ.get('CHROMIUM_EMAIL_ADDRESS',
                       os.environ.get('EMAIL_ADDRESS'))

and don't use ;

http://codereview.chromium.org/372048/diff/5/2001#newcode461
Line 461: "CHROMIUM_EMAIL_ADDRESS environment variable to set "
"one of TRY_EMAIL_ADDRESS or EMAIL_ADDRESS is set" otherwise it's misleading.

[Finnur, 2009-11-08 18:55:52]

Thanks for the comments. I have updated the patch.

http://codereview.chromium.org/372048/diff/5/2001
File trychange.py (right):

http://codereview.chromium.org/372048/diff/5/2001#newcode455
Line 455: email = os.environ.get('CHROMIUM_EMAIL_ADDRESS');
It didn't work because I made an error when copying it and didn't notice it when
reviewing it. It works now.

I opted for using TRYBOT_RESULTS_EMAIL_ADDRESS. I find that it describes the
value best.

[Mark Mentovai, 2009-11-08 19:04:48]

LG.

Your copy-paste error might have actually been my error, I left a single-quote
out of the sample I gave you.

http://codereview.chromium.org/372048/diff/4002/1003
File trychange.py (right):

http://codereview.chromium.org/372048/diff/4002/1003#newcode457
Line 457: os.environ.get('EMAIL_ADDRESS')),
Extra indent here to make it clear that it's part of the junk above and not a
new parameter to add_option?

http://codereview.chromium.org/372048/diff/4002/1003#newcode596
Line 596: print('Warning: TRYBOT_RESULTS_EMAIL_ADDRESS is not set. Try server '
print<space>('Warning:...

[M-A Ruel, 2009-11-08 20:18:42]

lgtm

http://codereview.chromium.org/372048/diff/4002/1003
File trychange.py (right):

http://codereview.chromium.org/372048/diff/4002/1003#newcode596
Line 596: print('Warning: TRYBOT_RESULTS_EMAIL_ADDRESS is not set. Try server '
On 2009/11/08 19:04:48, Mark Mentovai wrote:
> print<space>('Warning:...

Why? This fine fine as-is.

[Finnur, 2009-11-08 20:25:11]

I addressed the first issue (indentation) and left the space-before-parentheses
one alone. I did add parentheses though to the print function in the else
clause, to be consistent.

I have updated the patch and will commit Monday morning (don't have time to
address issues that might come up today).

Thanks for reviewing.

On 2009/11/08 20:18:42, Marc-Antoine Ruel wrote:
> lgtm
> 
> http://codereview.chromium.org/372048/diff/4002/1003
> File trychange.py (right):
> 
> http://codereview.chromium.org/372048/diff/4002/1003#newcode596
> Line 596: print('Warning: TRYBOT_RESULTS_EMAIL_ADDRESS is not set. Try server
'
> On 2009/11/08 19:04:48, Mark Mentovai wrote:
> > print<space>('Warning:...
> 
> Why? This fine fine as-is.

[M-A Ruel, 2009-11-09 16:40:08]

still lgtm

http://codereview.chromium.org/372048/diff/4007/1004
File trychange.py (right):

http://codereview.chromium.org/372048/diff/4007/1004#newcode457
Line 457: os.environ.get('EMAIL_ADDRESS')),
actually, this line should be +4 but not a big deal.

[Finnur, 2009-11-09 16:46:04]

I keep getting:

svn: Commit failed (details follow):
svn: Server sent unexpected return value (500 Internal Server Error) in
response
 to MKACTIVITY request for
'/svn/!svn/act/61f43495-24e2-af4a-a893-6d149f0a4727'

... when I try to commit. Any idea if this is a problem on the server or
locally?

On Mon, Nov 9, 2009 at 08:40, <maruel@chromium.org> wrote:

> still lgtm
>
>
> http://codereview.chromium.org/372048/diff/4007/1004
> File trychange.py (right):
>
> http://codereview.chromium.org/372048/diff/4007/1004#newcode457
>
> Line 457: os.environ.get('EMAIL_ADDRESS')),
> actually, this line should be +4 but not a big deal.
>
>
> http://codereview.chromium.org/372048
>

[M-A Ruel, 2009-11-09 16:58:06]

On 2009/11/09 16:46:04, Finnur wrote:
> I keep getting:
> 
> svn: Commit failed (details follow):
> svn: Server sent unexpected return value (500 Internal Server Error) in
> response
>  to MKACTIVITY request for
> '/svn/!svn/act/61f43495-24e2-af4a-a893-6d149f0a4727'
> 
> ... when I try to commit. Any idea if this is a problem on the server or
> locally?

Eh, switch to a svn:// checkout :)
You have a http:// checkout. Checkout the same directory in svn:// and copy the
depot_tools\.svn\gcl_info into the new checkout, along with the modified files.

[Finnur, 2009-11-09 17:06:38]

Ah!

Switched and checked in (r31437).

Thanks!

On 2009/11/09 16:58:06, Marc-Antoine Ruel wrote:
> On 2009/11/09 16:46:04, Finnur wrote:
> > I keep getting:
> > 
> > svn: Commit failed (details follow):
> > svn: Server sent unexpected return value (500 Internal Server Error) in
> > response
> >  to MKACTIVITY request for
> > '/svn/!svn/act/61f43495-24e2-af4a-a893-6d149f0a4727'
> > 
> > ... when I try to commit. Any idea if this is a problem on the server or
> > locally?
> 
> Eh, switch to a svn:// checkout :)
> You have a http:// checkout. Checkout the same directory in svn:// and copy
the
> depot_tools\.svn\gcl_info into the new checkout, along with the modified
files.