Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2377113003:
Add presubmit format requirement for webrtc/api/android (Closed)
|
Created:
4 years, 2 months ago by magjed_webrtc Modified:
4 years, 2 months ago CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionAdd presubmit format requirement for webrtc/api/android
BUG=webrtc:6419
NOTRY=True
TBR=kjellander@webrtc.org
Committed: https://crrev.com/606018600e1a6162ff343af10b706d62dba705a2
Cr-Commit-Position: refs/heads/master@{#14435}
Patch Set 1 #
Messages
Total messages: 16 (6 generated)
Description was changed from ========== Add presubmit format requirement for webrtc/api/android BUG=webrtc:6419 ========== to ========== Add presubmit format requirement for webrtc/api/android BUG=webrtc:6419 NOTRY=True ==========
magjed@webrtc.org changed reviewers: + kjellander@webrtc.org, sakal@webrtc.org
Please take a look.
Google Python style guide says we should lower case with underscores for function names but I guess it is better to use camel case here for consistency. lgtm
Or maybe we follow Chromium Python style guide, it says we use camel case on function names?
Or maybe we follow Chromium Python style guide, it says we use camel case on function names?
Description was changed from ========== Add presubmit format requirement for webrtc/api/android BUG=webrtc:6419 NOTRY=True ========== to ========== Add presubmit format requirement for webrtc/api/android BUG=webrtc:6419 NOTRY=True TBR=kjellander@webrtc.org ==========
On 2016/09/29 11:24:37, sakal wrote: > Or maybe we follow Chromium Python style guide, it says we use camel case on > function names? Yes, I think we follow Chromium Python style guide, it looks like that in the other WebRTC presubmit files as well. This code is more or less copy pasted from other Chromium presubmits.
The CQ bit was checked by magjed@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Message was sent while issue was closed.
Description was changed from ========== Add presubmit format requirement for webrtc/api/android BUG=webrtc:6419 NOTRY=True TBR=kjellander@webrtc.org ========== to ========== Add presubmit format requirement for webrtc/api/android BUG=webrtc:6419 NOTRY=True TBR=kjellander@webrtc.org ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Add presubmit format requirement for webrtc/api/android BUG=webrtc:6419 NOTRY=True TBR=kjellander@webrtc.org ========== to ========== Add presubmit format requirement for webrtc/api/android BUG=webrtc:6419 NOTRY=True TBR=kjellander@webrtc.org Committed: https://crrev.com/606018600e1a6162ff343af10b706d62dba705a2 Cr-Commit-Position: refs/heads/master@{#14435} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/606018600e1a6162ff343af10b706d62dba705a2 Cr-Commit-Position: refs/heads/master@{#14435}
Message was sent while issue was closed.
Regarding this change I'd prefer to see an effort in the top-level PRESUBMIT.py file instead, with a list of directories as we have for cpplint: https://chromium.googlesource.com/external/webrtc/+/master/PRESUBMIT.py That will make it easier to roll out such an effort project-wide. But for now I guess this is OK since it suits your needs. On 2016/09/29 11:42:43, magjed_webrtc wrote: > On 2016/09/29 11:24:37, sakal wrote: > > Or maybe we follow Chromium Python style guide, it says we use camel case on > > function names? > > Yes, I think we follow Chromium Python style guide, it looks like that in the > other WebRTC presubmit files as well. This code is more or less copy pasted from > other Chromium presubmits. Exactly, as written at http://dev.chromium.org/chromium-os/python-style-guidelines#TOC-Official-Styl...: Note that we deviate from that guide in two ways (in order to match the internal Google Python Style guide): * Indentation: use 2, not 4, spaces * Naming: Functions and Method names use CapWords() style, not lower_with_under() * The only exception to this rule is the main() function -- we do not use Main()
Message was sent while issue was closed.
On 2016/09/29 12:39:33, kjellander_webrtc wrote: > Regarding this change I'd prefer to see an effort in the top-level PRESUBMIT.py > file instead, with a list of directories as we have for cpplint: > https://chromium.googlesource.com/external/webrtc/+/master/PRESUBMIT.py > That will make it easier to roll out such an effort project-wide. > > But for now I guess this is OK since it suits your needs. Yeah that sounds like a good idea. I can help refactor once we add more directories. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
