Gerrit tryjob: !tryjob can be anywhere

Made Gerrit tryjob message format less restrictive

Any Gerrit message containing !tryjob is considered a try job. Anything
after !tryjob till the EOL/EOF is considered job definition and should
be in JSON.

R=kmg@chromium.org, vadimsh@chromium.org

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=266996

[Vadim Sh., 2014-04-29 21:36:48]

https://codereview.chromium.org/259323003/diff/1/scripts/master/try_job_gerri...
File scripts/master/try_job_gerrit.py (right):

https://codereview.chromium.org/259323003/diff/1/scripts/master/try_job_gerri...
scripts/master/try_job_gerrit.py:58: MESSAGE_REGEX_TRYJOB = re.compile('Patch
set \d+:( Tryjob\+1)?\s+!tryjob(.*)',
Take care of Code-Review and Verified labels too then, since they also can be
set in same message.

Or just skip everything until you see !tryjob.

[nodir, 2014-04-29 21:49:46]

https://codereview.chromium.org/259323003/diff/1/scripts/master/try_job_gerri...
File scripts/master/try_job_gerrit.py (right):

https://codereview.chromium.org/259323003/diff/1/scripts/master/try_job_gerri...
scripts/master/try_job_gerrit.py:58: MESSAGE_REGEX_TRYJOB = re.compile('Patch
set \d+:( Tryjob\+1)?\s+!tryjob(.*)',
On 2014/04/29 21:36:49, Vadim Sh. wrote:
> Take care of Code-Review and Verified labels too then, since they also can be
> set in same message.

They shouldn't.
 
> Or just skip everything until you see !tryjob.

I thought of this. This allows anybody to submit a tryjob in any comment, like

Hey John, here is my change. PTAL. I am going to try on Master: !tryjob
["Master"]

But I'm not sure about this. What do you think?

[Vadim Sh., 2014-04-29 21:53:33]

On 2014/04/29 21:49:46, nodir wrote:
>
https://codereview.chromium.org/259323003/diff/1/scripts/master/try_job_gerri...
> File scripts/master/try_job_gerrit.py (right):
> 
>
https://codereview.chromium.org/259323003/diff/1/scripts/master/try_job_gerri...
> scripts/master/try_job_gerrit.py:58: MESSAGE_REGEX_TRYJOB = re.compile('Patch
> set \d+:( Tryjob\+1)?\s+!tryjob(.*)',
> On 2014/04/29 21:36:49, Vadim Sh. wrote:
> > Take care of Code-Review and Verified labels too then, since they also can
be
> > set in same message.
> 
> They shouldn't.
>  

Why? What prevents me from setting Code-Review +1 and TryJob +1 in a same
message?

> > Or just skip everything until you see !tryjob.
> 
> I thought of this. This allows anybody to submit a tryjob in any comment, like
> 
> Hey John, here is my change. PTAL. I am going to try on Master: !tryjob
> ["Master"]
> 
> But I'm not sure about this. What do you think?

What prevents you from "faking" TryJob +1 message? It's all nasty text parsing.

[nodir, 2014-04-29 21:53:48]

On 2014/04/29 21:49:46, nodir wrote:
>
https://codereview.chromium.org/259323003/diff/1/scripts/master/try_job_gerri...
> File scripts/master/try_job_gerrit.py (right):
> 
>
https://codereview.chromium.org/259323003/diff/1/scripts/master/try_job_gerri...
> scripts/master/try_job_gerrit.py:58: MESSAGE_REGEX_TRYJOB = re.compile('Patch
> set \d+:( Tryjob\+1)?\s+!tryjob(.*)',
> On 2014/04/29 21:36:49, Vadim Sh. wrote:
> > Take care of Code-Review and Verified labels too then, since they also can
be
> > set in same message.
> 
> They shouldn't.

Clarification: git-try will set these labels, but I agree checking for Verified,
etc is more general.

This is the question of how strict this check should be.

> > Or just skip everything until you see !tryjob.
> 
> I thought of this. This allows anybody to submit a tryjob in any comment, like
> 
> Hey John, here is my change. PTAL. I am going to try on Master: !tryjob
> ["Master"]
> 
> But I'm not sure about this. What do you think?

Actually this is not entirely true. Currently TryJobGerritPoller expects the
tryjob definition be form !tryjob to EOF. We could check if !tryjob is followed
with a parenthesis and find a matching one, but this is more complicated than it
should be

[nodir, 2014-04-29 22:37:39]

PTAL.

Updated description:

Made Gerrit tryjob message format less restrictive

Any Gerrit message containing !tryjob is considered a try job. Anything
after !tryjob till the EOL/EOF is considered job definition and should
be in JSON.

[Vadim Sh., 2014-04-29 22:44:36]

lgtm

[nodir, 2014-04-29 23:51:50]

The CQ bit was checked by nodir@chromium.org

[commit-bot: I haz the power, 2014-04-29 23:52:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nodir@chromium.org/259323003/10002

[commit-bot: I haz the power, 2014-04-29 23:53:24]

Change committed as 266996