aboutsummaryrefslogtreecommitdiff
path: root/style-guide.md
AgeCommit message (Collapse)Author
2020-06-03C++ style: We don't allow designated initializersKarl Wiberg
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}
2019-12-11Add guidance to style guide how to reference a bug in a TODODanil Chapovalov
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-09-16Update WebRTC's C++ style guide to reflect the switch to C++14.Mirko Bonadei
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-02-05Remove rule that discourages passing optional by const referenceDanil Chapovalov
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}
2018-11-08Add a style rule about not using const optional<T>& argumentsKarl Wiberg
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-08-27Add a list of allowed and disallowed Abseil thingsKarl Wiberg
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-03-28Add style guidance about forward declarations.Taylor Brandstetter
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-22Add style guide rules for std::bind and std::functionKarl Wiberg
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-15Style guide: State what version of C++ we should useKarl Wiberg
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-14Add style guide rule about paired .h and .cc filesKarl Wiberg
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-14Style guide: The source code has moved; update link to matchKarl Wiberg
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-01-10Revert "Revert "GN rtc_* templates: Set default visibility to webrtc_root + ↵Per Kjellander
"/*""" 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-10Revert "GN rtc_* templates: Set default visibility to webrtc_root + "/*""Per Kjellander
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-10GN rtc_* templates: Set default visibility to webrtc_root + "/*"Karl Wiberg
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}
2017-12-11Adding style guidance for adding signals to pure interfaces.Taylor Brandstetter
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-09-09Style guide: Attempt to make the L2 and L3 headings more visually distinctKarl Wiberg
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-09Style guide: Add sections for Java, Objective-C/C++, and PythonKarl Wiberg
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-08Style guide: Link to Chromium's GN style guideKarl Wiberg
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-06Style guide: Add guideline for ArrayViewKarl Wiberg
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-06Style guide: Add guideline for conditional compilationKarl Wiberg
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-05Style guide: Add text about C and non-compliant C++ codeKarl Wiberg
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-03Add a bare-bones C++ style guideKarl Wiberg
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}