[Extensions] Don't check extension histogram on commit

[Extensions] Don't check extension histogram on commit

The PRESUBMIT.py check for the extension_function_histogram_value should not
trigger on commit, since there *are* valid ways to modify the histogram (most
notably, by deleting api functions).  We should leave the warning on presubmit,
but remove the CQ block so that we don't have to use NOTRY with these.

BUG=468507

[Devlin, 2015-03-18 22:42:15]

rdevlin.cronin@chromium.org changed reviewers:
+ rockot@chromium.org

[Ken Rockot(use gerrit already), 2015-03-19 16:37:04]

I'm a little wary of this only because presubmit warnings get ignored sometimes
and it would be pretty bad if someone slipped through a change which mangles
histograms.

Aren't valid-but-presubmit-breaking changes limited only to renames? How often
do we really have to do those?

[Devlin, 2015-03-19 16:54:19]

On 2015/03/19 16:37:04, Ken Rockot wrote:
> I'm a little wary of this only because presubmit warnings get ignored
sometimes
> and it would be pretty bad if someone slipped through a change which mangles
> histograms.
> 
> Aren't valid-but-presubmit-breaking changes limited only to renames? How often
> do we really have to do those?

Limited to renames/deletes, yes.  And they are fairly rare, but not so rare that
it's not a pain to deal with them (especially when "dealing" with them involves
using a scary NOTRY).  I guess my thought was more that if someone is really
going to ignore the upload warnings (which are pretty big, and difficult to
miss), and then the histogram reviewer (extension_function_histogram_value
requires a specific histograms OWNER) is going to miss the fact that the enum
was inserted somewhere randomly, then the likelihood that they are going to pay
more attention to the presubmit is somewhat slim.

Of course, the better solutions may be to either let StrictEnumValueChecker know
about valid renames, or to enable a NOPRESUBMIT option on cls, so that the rest
of the bots get tried.

[Ken Rockot(use gerrit already), 2015-03-19 17:01:53]

On 2015/03/19 16:54:19, Devlin wrote:
> On 2015/03/19 16:37:04, Ken Rockot wrote:
> > I'm a little wary of this only because presubmit warnings get ignored
> sometimes
> > and it would be pretty bad if someone slipped through a change which mangles
> > histograms.
> > 
> > Aren't valid-but-presubmit-breaking changes limited only to renames? How
often
> > do we really have to do those?
> 
> Limited to renames/deletes, yes.  And they are fairly rare, but not so rare
that
> it's not a pain to deal with them (especially when "dealing" with them
involves
> using a scary NOTRY).  I guess my thought was more that if someone is really
> going to ignore the upload warnings (which are pretty big, and difficult to
> miss), and then the histogram reviewer (extension_function_histogram_value
> requires a specific histograms OWNER) is going to miss the fact that the enum
> was inserted somewhere randomly, then the likelihood that they are going to
pay
> more attention to the presubmit is somewhat slim.
> 
> Of course, the better solutions may be to either let StrictEnumValueChecker
know
> about valid renames, or to enable a NOPRESUBMIT option on cls, so that the
rest
> of the bots get tried.

I like the idea of adding a NOPRESUBMIT option. I do agree that NOTRY is a very
bad idea in these situations.

[Ken Rockot(use gerrit already), 2015-09-29 19:25:37]

rockot@chromium.org changed reviewers:
- rockot@chromium.org