Commit Graph

16 Commits

Author SHA1 Message Date
212a20604a Add style guidance about forward declarations.
We prefer the Google style guide over the chromium guide in this case:
avoid forward declarations whenever possible. We don't have the same
compilation time issue that chromium does, so we can afford to do this.

The majority of our code already follows this guideline, as far as I'm
aware, though some forward declarations are still used to resolve
circular dependencies.

Bug: None
Notry: true
Change-Id: I712e0361acf76217067b13b8b3e4bc931d2a0238
Reviewed-on: https://webrtc-review.googlesource.com/8800
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22662}
2018-03-28 20:58:27 +00:00
0d44625b8f Add style guide rules for std::bind and std::function
We keep std::bind forbidden just like before, but suggest another
alternative than the Chromium style guide does.

We allow std::function, overriding the Chromium style guide which
forbids it on grounds not really applicable to WebRTC.

Bug: none
Change-Id: Iad07485652064a67020a494466d2b212bad568c0
Notry: true
Reviewed-on: https://webrtc-review.googlesource.com/63028
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22553}
2018-03-22 08:53:23 +00:00
180d99281d Style guide: State what version of C++ we should use
Bug: none
Notry: true
Change-Id: Id2a6d728479f4aeb5beff3fd594d95d565500bb6
Reviewed-on: https://webrtc-review.googlesource.com/61423
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#22440}
2018-03-15 10:32:16 +00:00
08c5cb0752 Add style guide rule about paired .h and .cc files
Bug: none
Notry: true
Change-Id: I26074f1decd81bae3c1045df5060c0c507c38a2d
Reviewed-on: https://webrtc-review.googlesource.com/59141
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22424}
2018-03-14 13:02:35 +00:00
3b4c590188 Style guide: The source code has moved; update link to match
Bug: none
Change-Id: I89a8451f36fe159ad18d0083ac3ce38004973d80
Notry: true
Reviewed-on: https://webrtc-review.googlesource.com/61721
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22422}
2018-03-14 12:18:15 +00:00
a7f2d84ad1 Revert "Revert "GN rtc_* templates: Set default visibility to webrtc_root + "/*"""
This reverts commit c73e1f437889d882cbf2987f7fb3a029a6150613.

Reason for revert: 
The problem with failed deps in chrome content/renderer had already been fixed in https://webrtc-review.googlesource.com/c/src/+/38660

Original change's description:
> Revert "GN rtc_* templates: Set default visibility to webrtc_root + "/*""
> 
> This reverts commit 588c548657b3ddf76e7b3f241263eef7f5799f16.
> 
> Reason for revert: 
> 
> Breaks Chrome FYI:
> 
> /b/c/b/Linux_Builder/src/buildtools/linux64/gn gen //out/Release --check
>   -> returned 1
> ERROR at //build/split_static_library.gni:12:5: Dependency not allowed.
>     static_library(target_name) {
>     ^----------------------------
> The item //content/renderer:renderer
> can not depend on //third_party/webrtc/media:rtc_internal_video_codecs
> because it is not in //third_party/webrtc/media:rtc_internal_video_codecs's visibility list: [
>   //third_party/webrtc/*
>   //third_party/webrtc_overrides/*
> ]
> 
>  https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.webrtc.fyi%2FLinux_Builder%2F23560%2F%2B%2Frecipes%2Fsteps%2Fgenerate_build_files%2F0%2Fstdout
> 
> Original change's description:
> > GN rtc_* templates: Set default visibility to webrtc_root + "/*"
> > 
> > This means that by default, targets are visible to everything under
> > the WebRTC root, but not visible to anything else.
> > 
> > API targets are manually tagged with visibility "*", so that targets
> > outside the WebRTC tree can see them.
> > 
> > BUG=webrtc:8254
> > 
> > Change-Id: Icdbee6e0d22d93240ff2fb530c8f9dc48e351509
> > Reviewed-on: https://webrtc-review.googlesource.com/24140
> > Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> > Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#21548}
> 
> TBR=mbonadei@webrtc.org,kwiberg@webrtc.org
> 
> Change-Id: I06620ce3d6f67482935c22efa231dd6cab91625a
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: webrtc:8254
> Reviewed-on: https://webrtc-review.googlesource.com/38760
> Reviewed-by: Per Kjellander <perkj@webrtc.org>
> Commit-Queue: Per Kjellander <perkj@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#21555}

TBR=mbonadei@webrtc.org,kwiberg@webrtc.org,perkj@webrtc.org

Change-Id: I6f720078ce21bd172e0a6471bae8c4c011e4a657
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:8254
Reviewed-on: https://webrtc-review.googlesource.com/38860
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21558}
2018-01-10 15:55:04 +00:00
c73e1f4378 Revert "GN rtc_* templates: Set default visibility to webrtc_root + "/*""
This reverts commit 588c548657b3ddf76e7b3f241263eef7f5799f16.

Reason for revert: 

Breaks Chrome FYI:

/b/c/b/Linux_Builder/src/buildtools/linux64/gn gen //out/Release --check
  -> returned 1
ERROR at //build/split_static_library.gni:12:5: Dependency not allowed.
    static_library(target_name) {
    ^----------------------------
The item //content/renderer:renderer
can not depend on //third_party/webrtc/media:rtc_internal_video_codecs
because it is not in //third_party/webrtc/media:rtc_internal_video_codecs's visibility list: [
  //third_party/webrtc/*
  //third_party/webrtc_overrides/*
]

 https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.webrtc.fyi%2FLinux_Builder%2F23560%2F%2B%2Frecipes%2Fsteps%2Fgenerate_build_files%2F0%2Fstdout

Original change's description:
> GN rtc_* templates: Set default visibility to webrtc_root + "/*"
> 
> This means that by default, targets are visible to everything under
> the WebRTC root, but not visible to anything else.
> 
> API targets are manually tagged with visibility "*", so that targets
> outside the WebRTC tree can see them.
> 
> BUG=webrtc:8254
> 
> Change-Id: Icdbee6e0d22d93240ff2fb530c8f9dc48e351509
> Reviewed-on: https://webrtc-review.googlesource.com/24140
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#21548}

TBR=mbonadei@webrtc.org,kwiberg@webrtc.org

Change-Id: I06620ce3d6f67482935c22efa231dd6cab91625a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:8254
Reviewed-on: https://webrtc-review.googlesource.com/38760
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21555}
2018-01-10 15:14:54 +00:00
588c548657 GN rtc_* templates: Set default visibility to webrtc_root + "/*"
This means that by default, targets are visible to everything under
the WebRTC root, but not visible to anything else.

API targets are manually tagged with visibility "*", so that targets
outside the WebRTC tree can see them.

BUG=webrtc:8254

Change-Id: Icdbee6e0d22d93240ff2fb530c8f9dc48e351509
Reviewed-on: https://webrtc-review.googlesource.com/24140
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21548}
2018-01-10 13:08:11 +00:00
efc5fbd8e0 Adding style guidance for adding signals to pure interfaces.
Based on discussion in the webrtc-core group. Note that NONE of our
existing code does this (yet), but I plan to convert it over time when
convenient.

Bug: None
Change-Id: Ie808181915ea24483e0fd8fbb06273351ebe661d
Reviewed-on: https://webrtc-review.googlesource.com/8140
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21214}
2017-12-11 20:56:56 +00:00
e1d464ec7b Style guide: Attempt to make the L2 and L3 headings more visually distinct
The default style of the L2 and L3 headers are too similar, making it difficult to see which is which.

BUG=none
NOTRY=true

Change-Id: I0deede2ad5766db9b63baa48be8e4da4aba784ca
Reviewed-on: https://chromium-review.googlesource.com/657697
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#19759}
2017-09-09 03:52:23 +00:00
241d7107b2 Style guide: Add sections for Java, Objective-C/C++, and Python
They're just pointers to the corresponding Chromium or Google style
guides.

BUG=none
NOTRY=true

Change-Id: Ib808db1cc4fc4d8a4ea708e6ec1c92d6d219b78e
Reviewed-on: https://chromium-review.googlesource.com/657419
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Kári Tristan Helgason <kthelgason@webrtc.org>
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Reviewed-by: Niels Möller <nisse@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Cr-Commit-Position: refs/heads/master@{#19758}
2017-09-09 03:41:33 +00:00
91d0ab71ef Style guide: Link to Chromium's GN style guide
BUG=none
NOTRY=true

Change-Id: I26f2588ef4bfecb39ab0f491508fd21797a8be5c
Reviewed-on: https://chromium-review.googlesource.com/652607
Reviewed-by: Henrik Kjellander <kjellander@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Niels Möller <nisse@chromium.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#19740}
2017-09-08 10:24:16 +00:00
e1ed24196b Style guide: Add guideline for ArrayView
BUG=none
NOTRY=true

Change-Id: I25ac7da349246f6bf1441200f2a2136218e30591
Reviewed-on: https://chromium-review.googlesource.com/648982
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Niels Möller <nisse@chromium.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#19711}
2017-09-06 12:01:40 +00:00
ee3d760e45 Style guide: Add guideline for conditional compilation
BUG=none
NOTRY=true

Change-Id: Ibcc05462a25216e8a2f30c9fd252239ba21693d4
Reviewed-on: https://chromium-review.googlesource.com/647551
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Niels Möller <nisse@chromium.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#19704}
2017-09-06 08:55:52 +00:00
fb4e677da0 Style guide: Add text about C and non-compliant C++ code
BUG=none
NOTRY=true

Change-Id: I64a64a4f138b6c8aa24ad2266284024d50163908
Reviewed-on: https://chromium-review.googlesource.com/648971
Reviewed-by: Niels Möller <nisse@chromium.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#19680}
2017-09-05 08:44:21 +00:00
bb821e28b1 Add a bare-bones C++ style guide
BUG=none
NOTRY=true

Change-Id: I4488aff47db2b9220985156c5112db6d5a79a18e
Reviewed-on: https://chromium-review.googlesource.com/641872
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#19645}
2017-09-03 16:43:41 +00:00