Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2869273003:
Add PRESUBMIT check to enforce the rule of tools/perf/contrib/ directory (Closed)
DescriptionAdd PRESUBMIT check to enforce the rule of tools/perf/contrib/ directory
This PRESUBMIT check makes sure:
1) All new files to tools/perf/contrib/ must belong to some subdirectory.
2) All the subdirectory of tools/perf/contrib/ must have at least 2 owners.
BUG=709063
Review-Url: https://codereview.chromium.org/2869273003
Cr-Commit-Position: refs/heads/master@{#470900}
Committed: https://chromium.googlesource.com/chromium/src/+/aa3196de10db663af40f600d476dfb77587994c3
Patch Set 1 #
Total comments: 3
Patch Set 2 : update #Patch Set 3 : remove debug print #Messages
Total messages: 23 (13 generated)
nednguyen@google.com changed reviewers: + charliea@chromium.org, sullivan@chromium.org
The CQ bit was checked by nednguyen@google.com 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...
lgtm https://codereview.chromium.org/2869273003/diff/1/tools/perf/PRESUBMIT.py File tools/perf/PRESUBMIT.py (right): https://codereview.chromium.org/2869273003/diff/1/tools/perf/PRESUBMIT.py#new... tools/perf/PRESUBMIT.py:68: return results Just wanted to check there's not already an API in presubmit for listing owners?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2869273003/diff/1/tools/perf/PRESUBMIT.py File tools/perf/PRESUBMIT.py (right): https://codereview.chromium.org/2869273003/diff/1/tools/perf/PRESUBMIT.py#new... tools/perf/PRESUBMIT.py:71: def _CheckContribDir(input_api, output_api): (I'm not super familiar with how PRESUBMIT.py works, so I apologize if this is a dumb question...) Could we just put this in a PRESUBMIT.py in the contrib/ directory, given that it only applies to that directory? I'm guessing that the actual presubmit interface revolves around CheckChangeOnUpload and CheckChangeOnCommit, both of which have a pretty simple interface, so it's not like we're taking advantage of a bunch of common code by putting it in this file.
On 2017/05/10 15:52:30, charliea wrote: > https://codereview.chromium.org/2869273003/diff/1/tools/perf/PRESUBMIT.py > File tools/perf/PRESUBMIT.py (right): > > https://codereview.chromium.org/2869273003/diff/1/tools/perf/PRESUBMIT.py#new... > tools/perf/PRESUBMIT.py:71: def _CheckContribDir(input_api, output_api): > (I'm not super familiar with how PRESUBMIT.py works, so I apologize if this is a > dumb question...) > > Could we just put this in a PRESUBMIT.py in the contrib/ directory, given that > it only applies to that directory? Great idea. > > I'm guessing that the actual presubmit interface revolves around > CheckChangeOnUpload and CheckChangeOnCommit, both of which have a pretty simple > interface, so it's not like we're taking advantage of a bunch of common code by > putting it in this file.
Moved PRESUBMIT.py to tools/perf/contrib/ PTAL again https://codereview.chromium.org/2869273003/diff/1/tools/perf/PRESUBMIT.py File tools/perf/PRESUBMIT.py (right): https://codereview.chromium.org/2869273003/diff/1/tools/perf/PRESUBMIT.py#new... tools/perf/PRESUBMIT.py:68: return results On 2017/05/10 14:59:19, sullivan wrote: > Just wanted to check there's not already an API in presubmit for listing owners? I look around and there is no API there that support this.
lgtm
The CQ bit was checked by nednguyen@google.com
The CQ bit was unchecked by nednguyen@google.com
The CQ bit was checked by nednguyen@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from sullivan@chromium.org Link to the patchset: https://codereview.chromium.org/2869273003/#ps40001 (title: "remove debug print")
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: android_compile_dbg on master.tryserver.chromium.android (JOB_TIMED_OUT, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...) android_cronet on master.tryserver.chromium.android (JOB_TIMED_OUT, https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_TIMED_OUT, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
The CQ bit was checked by nednguyen@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": 40001, "attempt_start_ts": 1494496621655780,
"parent_rev": "111fffdac5769c2358cd7e1d272529bbcca41700", "commit_rev":
"aa3196de10db663af40f600d476dfb77587994c3"}
Message was sent while issue was closed.
Description was changed from ========== Add PRESUBMIT check to enforce the rule of tools/perf/contrib/ directory This PRESUBMIT check makes sure: 1) All new files to tools/perf/contrib/ must belong to some subdirectory. 2) All the subdirectory of tools/perf/contrib/ must have at least 2 owners. BUG=709063 ========== to ========== Add PRESUBMIT check to enforce the rule of tools/perf/contrib/ directory This PRESUBMIT check makes sure: 1) All new files to tools/perf/contrib/ must belong to some subdirectory. 2) All the subdirectory of tools/perf/contrib/ must have at least 2 owners. BUG=709063 Review-Url: https://codereview.chromium.org/2869273003 Cr-Commit-Position: refs/heads/master@{#470900} Committed: https://chromium.googlesource.com/chromium/src/+/aa3196de10db663af40f600d476d... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/aa3196de10db663af40f600d476d... |
