Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2797993004:
Configure components/viz/ directory. (Closed)
DescriptionConfigure components/viz/ directory.
Add to OWNERS. Add a PRESUBMIT.py that is similar to cc/PRESUBMIT.py.
BUG=none
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
Review-Url: https://codereview.chromium.org/2797993004
Cr-Commit-Position: refs/heads/master@{#462142}
Committed: https://chromium.googlesource.com/chromium/src/+/e68cc4a0cef3f755dcebcb1cad23560a10c34f7f
Patch Set 1 #Patch Set 2 : Fix presubmit. #
Total comments: 8
Patch Set 3 : Fixes. #
Total comments: 10
Patch Set 4 : More fixes. #Patch Set 5 : Typo. #
Messages
Total messages: 28 (14 generated)
kylechar@chromium.org changed reviewers: + danakj@chromium.org, fsamuel@chromium.org
Description was changed from ========== Top level components/viz/ setup. Add to OWNERS. Add a PRESUBMIT.py that is similar. BUG=none ========== to ========== Configure components/viz/ directory. Add to OWNERS. Add a PRESUBMIT.py that is similar to cc/PRESUBMIT.py. BUG=none ==========
https://codereview.chromium.org/2797993004/diff/20001/components/viz/OWNERS File components/viz/OWNERS (right): https://codereview.chromium.org/2797993004/diff/20001/components/viz/OWNERS#n... components/viz/OWNERS:5: danakj@chromium.org OOH there's already an owners here. I didn't see that happen, that's why I was suggesting it in the other CL. Maybe add other cc/ owners that review/work on this stuff? I had thought enne,vmpstr,piman,jbauman?
LGTM https://codereview.chromium.org/2797993004/diff/20001/components/viz/PRESUBMI... File components/viz/PRESUBMIT.py (right): https://codereview.chromium.org/2797993004/diff/20001/components/viz/PRESUBMI... components/viz/PRESUBMIT.py:35: # WebKit ASSERT_NOT_REACHED() is not allowed. ASSERT_NOT_REACHED() has gone away now, so this could be removed (in cc also) https://codereview.chromium.org/2797993004/diff/20001/components/viz/PRESUBMI... components/viz/PRESUBMIT.py:106: # Well-defined simple classes containing only <= 4 ints, or <= 2 floats. This is actually now types the same size as a primitive type, could you say that instead (in cc too)? https://codereview.chromium.org/2797993004/diff/20001/components/viz/PRESUBMI... components/viz/PRESUBMIT.py:134: 'All TODO comments should be of the form TODO(name). ' + TODO(name/bug) is the new format, can you say that (and in cc too)
It looks like rietveld won't pick up similar python files no matter what I set the threshold too. I changed some checks for "cc" to "components/viz" or "viz" depending. I also removed the extra blink CQ bots being added in the post presubmit. Otherwise it is unchanged.
Description was changed from ========== Configure components/viz/ directory. Add to OWNERS. Add a PRESUBMIT.py that is similar to cc/PRESUBMIT.py. BUG=none ========== to ========== Configure components/viz/ directory. Add to OWNERS. Add a PRESUBMIT.py that is similar to cc/PRESUBMIT.py. BUG=none CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
https://codereview.chromium.org/2797993004/diff/20001/components/viz/OWNERS File components/viz/OWNERS (right): https://codereview.chromium.org/2797993004/diff/20001/components/viz/OWNERS#n... components/viz/OWNERS:5: danakj@chromium.org On 2017/04/05 16:38:07, danakj wrote: > OOH there's already an owners here. I didn't see that happen, that's why I was > suggesting it in the other CL. > > Maybe add other cc/ owners that review/work on this stuff? I had thought > enne,vmpstr,piman,jbauman? Done. Talked to fsamuel offline and I've just added everyone explicitly instead of copying what services/ui/gpu/OWNERS had. https://codereview.chromium.org/2797993004/diff/20001/components/viz/PRESUBMI... File components/viz/PRESUBMIT.py (right): https://codereview.chromium.org/2797993004/diff/20001/components/viz/PRESUBMI... components/viz/PRESUBMIT.py:35: # WebKit ASSERT_NOT_REACHED() is not allowed. On 2017/04/05 16:41:30, danakj wrote: > ASSERT_NOT_REACHED() has gone away now, so this could be removed (in cc also) Done. https://codereview.chromium.org/2797993004/diff/20001/components/viz/PRESUBMI... components/viz/PRESUBMIT.py:106: # Well-defined simple classes containing only <= 4 ints, or <= 2 floats. On 2017/04/05 16:41:30, danakj wrote: > This is actually now types the same size as a primitive type, could you say that > instead (in cc too)? Done. https://codereview.chromium.org/2797993004/diff/20001/components/viz/PRESUBMI... components/viz/PRESUBMIT.py:134: 'All TODO comments should be of the form TODO(name). ' + On 2017/04/05 16:41:30, danakj wrote: > TODO(name/bug) is the new format, can you say that (and in cc too) Done.
Thanks! LGTM https://codereview.chromium.org/2797993004/diff/40001/cc/PRESUBMIT.py File cc/PRESUBMIT.py (left): https://codereview.chromium.org/2797993004/diff/40001/cc/PRESUBMIT.py#oldcode28 cc/PRESUBMIT.py:28: notreached_files = [] this goes https://codereview.chromium.org/2797993004/diff/40001/cc/PRESUBMIT.py#oldcode43 cc/PRESUBMIT.py:43: if notreached_files: this goes https://codereview.chromium.org/2797993004/diff/40001/components/viz/PRESUBMI... File components/viz/PRESUBMIT.py (right): https://codereview.chromium.org/2797993004/diff/40001/components/viz/PRESUBMI... components/viz/PRESUBMIT.py:28: notreached_files = [] same https://codereview.chromium.org/2797993004/diff/40001/components/viz/PRESUBMI... components/viz/PRESUBMIT.py:40: if notreached_files: same
https://codereview.chromium.org/2797993004/diff/40001/cc/PRESUBMIT.py File cc/PRESUBMIT.py (left): https://codereview.chromium.org/2797993004/diff/40001/cc/PRESUBMIT.py#oldcode28 cc/PRESUBMIT.py:28: notreached_files = [] On 2017/04/05 16:59:41, danakj wrote: > this goes Done. https://codereview.chromium.org/2797993004/diff/40001/cc/PRESUBMIT.py#oldcode43 cc/PRESUBMIT.py:43: if notreached_files: On 2017/04/05 16:59:41, danakj wrote: > this goes Done. https://codereview.chromium.org/2797993004/diff/40001/components/viz/PRESUBMI... File components/viz/PRESUBMIT.py (right): https://codereview.chromium.org/2797993004/diff/40001/components/viz/PRESUBMI... components/viz/PRESUBMIT.py:28: notreached_files = [] On 2017/04/05 16:59:41, danakj wrote: > same Done. https://codereview.chromium.org/2797993004/diff/40001/components/viz/PRESUBMI... components/viz/PRESUBMIT.py:40: if notreached_files: On 2017/04/05 16:59:41, danakj wrote: > same Done.
https://codereview.chromium.org/2797993004/diff/40001/cc/PRESUBMIT.py File cc/PRESUBMIT.py (right): https://codereview.chromium.org/2797993004/diff/40001/cc/PRESUBMIT.py#newcode103 cc/PRESUBMIT.py:103: # Well-defined simple classes the same size as a primative type. nit: primitive
https://codereview.chromium.org/2797993004/diff/40001/cc/PRESUBMIT.py File cc/PRESUBMIT.py (right): https://codereview.chromium.org/2797993004/diff/40001/cc/PRESUBMIT.py#newcode103 cc/PRESUBMIT.py:103: # Well-defined simple classes the same size as a primative type. On 2017/04/05 17:09:14, Fady Samuel wrote: > nit: primitive Done.
lgtm
The CQ bit was checked by kylechar@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org Link to the patchset: https://codereview.chromium.org/2797993004/#ps80001 (title: "Typo.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...) win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...) win_clang on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
The CQ bit was checked by kylechar@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was unchecked by kylechar@chromium.org
The CQ bit was checked by kylechar@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1491414581259950,
"parent_rev": "90acaf829ceba5299b4823723f44ac72bfabf3d8", "commit_rev":
"e68cc4a0cef3f755dcebcb1cad23560a10c34f7f"}
Message was sent while issue was closed.
Description was changed from ========== Configure components/viz/ directory. Add to OWNERS. Add a PRESUBMIT.py that is similar to cc/PRESUBMIT.py. BUG=none CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== Configure components/viz/ directory. Add to OWNERS. Add a PRESUBMIT.py that is similar to cc/PRESUBMIT.py. BUG=none CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2797993004 Cr-Commit-Position: refs/heads/master@{#462142} Committed: https://chromium.googlesource.com/chromium/src/+/e68cc4a0cef3f755dcebcb1cad23... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/e68cc4a0cef3f755dcebcb1cad23... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
