Adding support for NOTREECHECKS

Adding support for NOTREECHECKS.

The motivation for this CL is https://codereview.appspot.com/22410044/ ('One-Click Revert in Rietveld'). One of the main use-cases of One-Click revert is to be able to do a quick revert of a breaking change when the tree is closed.

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

[iannucci, 2013-11-13 06:24:22]

lgtm % comments

https://codereview.chromium.org/68113009/diff/30001/verification/tree_status.py
File verification/tree_status.py (right):

https://codereview.chromium.org/68113009/diff/30001/verification/tree_status....
verification/tree_status.py:67: return match and match.group(1).lower() ==
'true'
I'm not sure why this function can't be inlined as 

if re.search(r'(?m)^NOTREECHECKS=[tT][rR][uU][eE]', description):
  ...

You could even put the regex as a module-level constant using re.compile()

if you're flexible on the flag itself, the regex could just be

r'(?im)^NOTREECHECKS=TRUE'

https://codereview.chromium.org/68113009/diff/30001/verification/tree_status....
verification/tree_status.py:81: description=pending.description)
Another way to do it would be to process description and just store a bool. Then
the cq's json file won't bloat to contain a copy of the description just so we
can regex it later.

[iannucci, 2013-11-13 06:25:15]

https://codereview.chromium.org/68113009/diff/30001/verification/tree_status.py
File verification/tree_status.py (right):

https://codereview.chromium.org/68113009/diff/30001/verification/tree_status....
verification/tree_status.py:81: description=pending.description)
On 2013/11/13 06:24:22, iannucci wrote:
> Another way to do it would be to process description and just store a bool.
Then
> the cq's json file won't bloat to contain a copy of the description just so we
> can regex it later.

Actually... we could just do the description processing here and set
pending.verifications[self.name] to a bogus TreeStatus class that just returns
False for pending()

[rmistry, 2013-11-13 19:44:43]

https://codereview.chromium.org/68113009/diff/30001/verification/tree_status.py
File verification/tree_status.py (right):

https://codereview.chromium.org/68113009/diff/30001/verification/tree_status....
verification/tree_status.py:67: return match and match.group(1).lower() ==
'true'
On 2013/11/13 06:24:22, iannucci wrote:
> I'm not sure why this function can't be inlined as 

No reason, this was just exactly how try_job_on_rietveld.py did it.

> 
> if re.search(r'(?m)^NOTREECHECKS=[tT][rR][uU][eE]', description):
>   ...
> 
> You could even put the regex as a module-level constant using re.compile()

I think using a precompiled module-level constant makes more sense here if it
was used in multiple places in the code. Also I believe python compiles and
caches regexes whenever they are used anyway.

> 
> if you're flexible on the flag itself, the regex could just be
> 
> r'(?im)^NOTREECHECKS=TRUE'

Done.

https://codereview.chromium.org/68113009/diff/30001/verification/tree_status....
verification/tree_status.py:81: description=pending.description)
On 2013/11/13 06:25:15, iannucci wrote:
> On 2013/11/13 06:24:22, iannucci wrote:
> > Another way to do it would be to process description and just store a bool.
> Then
> > the cq's json file won't bloat to contain a copy of the description just so
we
> > can regex it later.
> 
> Actually... we could just do the description processing here and set
> pending.verifications[self.name] to a bogus TreeStatus class that just returns
> False for pending()

SGTM. Done. PTAL.

[iannucci, 2013-11-14 07:40:50]

Nice. lgtm

https://codereview.chromium.org/68113009/diff/130001/tests/tree_status_test.py
File tests/tree_status_test.py (right):

https://codereview.chromium.org/68113009/diff/130001/tests/tree_status_test.p...
tests/tree_status_test.py:111: class dummy_pending:
should derive from object to be a 'new-style' class.

[rmistry, 2013-11-14 12:40:40]

https://codereview.chromium.org/68113009/diff/130001/tests/tree_status_test.py
File tests/tree_status_test.py (right):

https://codereview.chromium.org/68113009/diff/130001/tests/tree_status_test.p...
tests/tree_status_test.py:111: class dummy_pending:
On 2013/11/14 07:40:51, iannucci wrote:
> should derive from object to be a 'new-style' class.

Done.

[commit-bot: I haz the power, 2013-11-14 12:40:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rmistry@google.com/68113009/180001

[commit-bot: I haz the power, 2013-11-14 12:41:19]

Change committed as 235127

[rmistry, 2013-11-14 12:42:26]

Thanks Robbie, could you please resync and restart the Skia CQ?