enabling `gn check` on the whole WebRTC repo

enabling `gn check` on the whole WebRTC repo

BUG=webrtc:6828
NOTRY=True

Review-Url: https://codereview.webrtc.org/2918803002
Cr-Commit-Position: refs/heads/master@{#18390}
Committed: https://chromium.googlesource.com/external/webrtc/+/7d9a55b92db2f41b597a084e00853e017d94000f

[mbonadei, 2017-06-01 13:31:46]

mbonadei@webrtc.org changed reviewers:
+ kjellander@webrtc.org

[kjellander_webrtc, 2017-06-01 13:39:02]

https://codereview.webrtc.org/2918803002/diff/40001/webrtc/BUILD.gn
File webrtc/BUILD.gn (right):

https://codereview.webrtc.org/2918803002/diff/40001/webrtc/BUILD.gn#newcode378
webrtc/BUILD.gn:378: "base:rtc_base_approved",
Since we have the check disabled, don't you think it makes sense to remove this
too? Tons of target depends on webrtc_common...

[mbonadei, 2017-06-01 13:44:31]

https://codereview.webrtc.org/2918803002/diff/40001/webrtc/BUILD.gn
File webrtc/BUILD.gn (right):

https://codereview.webrtc.org/2918803002/diff/40001/webrtc/BUILD.gn#newcode378
webrtc/BUILD.gn:378: "base:rtc_base_approved",
On 2017/06/01 13:39:01, kjellander_webrtc wrote:
> Since we have the check disabled, don't you think it makes sense to remove
this
> too? Tons of target depends on webrtc_common...

I don't know... It is still an improvement respect to 0 dependencies.
Let me know what do you prefer, I don't have a strong opinion since
with check_includes = false things will probably move without any
notice.

[kjellander_webrtc, 2017-06-01 14:01:18]

https://codereview.webrtc.org/2918803002/diff/40001/webrtc/BUILD.gn
File webrtc/BUILD.gn (right):

https://codereview.webrtc.org/2918803002/diff/40001/webrtc/BUILD.gn#newcode378
webrtc/BUILD.gn:378: "base:rtc_base_approved",
On 2017/06/01 13:44:30, mbonadei wrote:
> On 2017/06/01 13:39:01, kjellander_webrtc wrote:
> > Since we have the check disabled, don't you think it makes sense to remove
> this
> > too? Tons of target depends on webrtc_common...
> 
> I don't know... It is still an improvement respect to 0 dependencies.
> Let me know what do you prefer, I don't have a strong opinion since
> with check_includes = false things will probably move without any
> notice.

I'm just thinking that since it's something that shouldn't be here in the first
place, maybe the person fixing the dependency forgets to remove it when doing
so.

[mbonadei, 2017-06-01 19:08:48]

https://codereview.webrtc.org/2918803002/diff/40001/webrtc/BUILD.gn
File webrtc/BUILD.gn (right):

https://codereview.webrtc.org/2918803002/diff/40001/webrtc/BUILD.gn#newcode378
webrtc/BUILD.gn:378: "base:rtc_base_approved",
On 2017/06/01 14:01:17, kjellander_webrtc wrote:
> On 2017/06/01 13:44:30, mbonadei wrote:
> > On 2017/06/01 13:39:01, kjellander_webrtc wrote:
> > > Since we have the check disabled, don't you think it makes sense to remove
> > this
> > > too? Tons of target depends on webrtc_common...
> > 
> > I don't know... It is still an improvement respect to 0 dependencies.
> > Let me know what do you prefer, I don't have a strong opinion since
> > with check_includes = false things will probably move without any
> > notice.
> 
> I'm just thinking that since it's something that shouldn't be here in the
first
> place, maybe the person fixing the dependency forgets to remove it when doing
> so.

I am a little bit confused. It is a real dependency (webrtc/config.h includes
webrtc/base/basictypes.h) so we will not remove it even when the cycle
will be fixed. Am I missing something? :)

[kjellander_webrtc, 2017-06-01 19:45:17]

lgtm

https://codereview.webrtc.org/2918803002/diff/40001/webrtc/BUILD.gn
File webrtc/BUILD.gn (right):

https://codereview.webrtc.org/2918803002/diff/40001/webrtc/BUILD.gn#newcode378
webrtc/BUILD.gn:378: "base:rtc_base_approved",
On 2017/06/01 19:08:48, mbonadei wrote:
> On 2017/06/01 14:01:17, kjellander_webrtc wrote:
> > On 2017/06/01 13:44:30, mbonadei wrote:
> > > On 2017/06/01 13:39:01, kjellander_webrtc wrote:
> > > > Since we have the check disabled, don't you think it makes sense to
remove
> > > this
> > > > too? Tons of target depends on webrtc_common...
> > > 
> > > I don't know... It is still an improvement respect to 0 dependencies.
> > > Let me know what do you prefer, I don't have a strong opinion since
> > > with check_includes = false things will probably move without any
> > > notice.
> > 
> > I'm just thinking that since it's something that shouldn't be here in the
> first
> > place, maybe the person fixing the dependency forgets to remove it when
doing
> > so.
> 
> I am a little bit confused. It is a real dependency (webrtc/config.h includes
> webrtc/base/basictypes.h) so we will not remove it even when the cycle
> will be fixed. Am I missing something? :)

Leaving it in fine. I just thought we were about to refactor it out, but it'll
probably take a while anyway.

[mbonadei, 2017-06-01 19:55:58]

https://codereview.webrtc.org/2918803002/diff/40001/webrtc/BUILD.gn
File webrtc/BUILD.gn (right):

https://codereview.webrtc.org/2918803002/diff/40001/webrtc/BUILD.gn#newcode378
webrtc/BUILD.gn:378: "base:rtc_base_approved",
On 2017/06/01 19:45:17, kjellander_webrtc wrote:
> On 2017/06/01 19:08:48, mbonadei wrote:
> > On 2017/06/01 14:01:17, kjellander_webrtc wrote:
> > > On 2017/06/01 13:44:30, mbonadei wrote:
> > > > On 2017/06/01 13:39:01, kjellander_webrtc wrote:
> > > > > Since we have the check disabled, don't you think it makes sense to
> remove
> > > > this
> > > > > too? Tons of target depends on webrtc_common...
> > > > 
> > > > I don't know... It is still an improvement respect to 0 dependencies.
> > > > Let me know what do you prefer, I don't have a strong opinion since
> > > > with check_includes = false things will probably move without any
> > > > notice.
> > > 
> > > I'm just thinking that since it's something that shouldn't be here in the
> > first
> > > place, maybe the person fixing the dependency forgets to remove it when
> doing
> > > so.
> > 
> > I am a little bit confused. It is a real dependency (webrtc/config.h
includes
> > webrtc/base/basictypes.h) so we will not remove it even when the cycle
> > will be fixed. Am I missing something? :)
> 
> Leaving it in fine. I just thought we were about to refactor it out, but it'll
> probably take a while anyway.

Ah ok, now I got it. I was missing that webrtc:5876 is about moving
webrtc_common to webrtc/api. :)

[mbonadei, 2017-06-01 19:59:00]

The CQ bit was checked by mbonadei@webrtc.org

[mbonadei, 2017-06-01 19:59:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
kjellander@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2918803002/#ps60001 (title:
"removing TODO")

[commit-bot: I haz the power, 2017-06-01 19:59:10]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-06-01 20:01:48]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1496347140915750,
"parent_rev": "3d4b83da91c269b685ff9df6d1bb1793270b3508", "commit_rev":
"7d9a55b92db2f41b597a084e00853e017d94000f"}

[commit-bot: I haz the power, 2017-06-01 20:01:52]

Description was changed from

==========
enabling `gn check` on the whole WebRTC repo

BUG=webrtc:6828
NOTRY=True
==========

to

==========
enabling `gn check` on the whole WebRTC repo

BUG=webrtc:6828
NOTRY=True

Review-Url: https://codereview.webrtc.org/2918803002
Cr-Commit-Position: refs/heads/master@{#18390}
Committed:
https://chromium.googlesource.com/external/webrtc/+/7d9a55b92db2f41b597a084e0...
==========

[commit-bot: I haz the power, 2017-06-01 20:01:53]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/external/webrtc/+/7d9a55b92db2f41b597a084e0...