Commit Graph

22 Commits

Author SHA1 Message Date
878808892d C++ style: We don't allow designated initializers
Bug: None
Change-Id: Ib23b72e89b1db1eb363f4f7da040045174dd70d7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176407
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31419}
2020-06-03 09:11:09 +00:00
375eff4f04 Add guidance to style guide how to reference a bug in a TODO
Bug: None
Change-Id: Icfbce347d0c2a71fd728507e5005eb05736b13a1
No-Try: True
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/161733
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30058}
2019-12-11 11:55:52 +00:00
1b6a30ddcc Update WebRTC's C++ style guide to reflect the switch to C++14.
No-Try: True
Bug: webrtc:10945
Change-Id: Ife5d5c12144e00aeefd5ccfe8470c8741ad8eb54
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/151460
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29194}
2019-09-16 11:45:35 +00:00
b769894025 Remove rule that discourages passing optional by const reference
include example to demonstrate:
(subjectively) increased readability
(objectively) decreased binary size

Bug: None
Change-Id: I970e668af98d98725b2d527f44169a8b7c9d2338
Reviewed-on: https://webrtc-review.googlesource.com/c/121420
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Johannes Kron <kron@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26545}
2019-02-05 11:58:05 +00:00
694ed1793c Add a style rule about not using const optional<T>& arguments
Motivated by discussions here:
https://webrtc-review.googlesource.com/c/src/+/109583

Bug: none
Change-Id: Ia0723adf9fa7c970137ffc9cb5612cb3360d7f5f
Notry: true
Reviewed-on: https://webrtc-review.googlesource.com/c/109568
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25556}
2018-11-08 11:57:35 +00:00
c3af97d68c Add a list of allowed and disallowed Abseil things
Bug: webrtc:8821
Change-Id: Ifb3bacd3403bbf823c78fff47571a83159f1da73
No-Try: True
Reviewed-on: https://webrtc-review.googlesource.com/95880
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24437}
2018-08-27 08:37:20 +00:00
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