Make DISALLOW_* presubmit stricter

Make DISALLOW_* presubmit stricter

R=danakj@chromium.org
BUG=none
TEST=less false positives

[danakj, 2015-12-04 01:06:21]

LGTM but I'd honestly prefer to remove the check

[Dan Beam, 2015-12-04 01:48:56]

On 2015/12/04 01:06:21, danakj (behind on reviews) wrote:
> LGTM but I'd honestly prefer to remove the check

then remove it?

[Dan Beam, 2015-12-04 02:44:49]

On 2015/12/04 01:48:56, Dan Beam wrote:
> On 2015/12/04 01:06:21, danakj (behind on reviews) wrote:
> > LGTM but I'd honestly prefer to remove the check
> 
> then remove it?

to make sure this isn't being misinterpreted as snarky: I think removing this
check is completely valid and I agree with your assessment that this is a really
really bad way to do IWYU.

I added this check to selfishly save myself time, but I figured it might help
others as well.  We already do tons of things resembling parsing C++ in
PRESUBMITs already.

I also just don't know any other concrete plans for a better IWYU system, but
you are the base/OWNER that would have to change this if you changed macros.h.

If our attitude is "if it compiles, it works", I'm fine with dropping this check
and not enforcing IWYU in reviews.

[danakj, 2015-12-04 21:36:54]

On 2015/12/04 02:44:49, Dan Beam wrote:
> On 2015/12/04 01:48:56, Dan Beam wrote:
> > On 2015/12/04 01:06:21, danakj (behind on reviews) wrote:
> > > LGTM but I'd honestly prefer to remove the check
> > 
> > then remove it?
> 
> to make sure this isn't being misinterpreted as snarky: I think removing this
> check is completely valid and I agree with your assessment that this is a
really
> really bad way to do IWYU.
> 
> I added this check to selfishly save myself time, but I figured it might help
> others as well.  We already do tons of things resembling parsing C++ in
> PRESUBMITs already.
> 
> I also just don't know any other concrete plans for a better IWYU system, but
> you are the base/OWNER that would have to change this if you changed macros.h.
> 
> If our attitude is "if it compiles, it works", I'm fine with dropping this
check
> and not enforcing IWYU in reviews.

IWYU is nice because it prevents changes in one file from breaking another place
via losing a transitive include. I'm okay with out best-effort practices until
we have a real IWYU checker, and for this header I'm especially okay with it
being transitively included, since it is basically everywhere and likelihood of
breaking is small. I appreciate your efforts though, and don't want to make
reviewing harder. I do think you could mostly not worry about checking for this
being included explicitly though.

[Nico, 2015-12-04 21:42:56]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2015-12-04 21:42:56]

https://codereview.chromium.org/1490383004/diff/1/PRESUBMIT.py
File PRESUBMIT.py (right):

https://codereview.chromium.org/1490383004/diff/1/PRESUBMIT.py#newcode1547
PRESUBMIT.py:1547: 'DISALLOW_EVIL_CONSTRUCTORS(',
fwiw i removed DISALLOW_EVIL_CTORS from macros.h earlier this morning, so maybe
this line can go now

[Dan Beam, 2015-12-04 22:53:02]

https://codereview.chromium.org/1490383004/diff/1/PRESUBMIT.py
File PRESUBMIT.py (right):

https://codereview.chromium.org/1490383004/diff/1/PRESUBMIT.py#newcode1547
PRESUBMIT.py:1547: 'DISALLOW_EVIL_CONSTRUCTORS(',
On 2015/12/04 21:42:56, Nico wrote:
> fwiw i removed DISALLOW_EVIL_CTORS from macros.h earlier this morning, so
maybe
> this line can go now

look down, look back

THE PRESUBMIT IS NOW GONE

https://youtu.be/owGykVbfgUE

[Dan Beam, 2015-12-04 22:53:22]

Description was changed from

==========
Make DISALLOW_* presubmit stricter

R=danakj@chromium.org
BUG=none
TEST=less false positives
==========

to

==========
Make DISALLOW_* presubmit stricter

R=danakj@chromium.org
BUG=none
TEST=less false positives
==========

[Dan Beam, 2015-12-04 22:56:39]

closing in favor of: https://codereview.chromium.org/1501793003/

[danakj, 2015-12-04 22:57:42]

LGTM