Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(27)

Issue 719463003: Presubmit checks for user actions intorduced in HTML files. (Closed)

Created:
6 years, 1 month ago by gayane -on leave until 09-2017
Modified:
6 years ago
CC:
chromium-reviews, Ilya Sherman, arv+watch_chromium.org, asvitkine+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Presubmit checks for user actions introduced in HTML files. This presubmit task will catch the case when HTML files are modified with new user action and action.xml is not updated BUG=431393 Committed: https://crrev.com/3dff8c2676a1585b5945a8671f97d0b1e2eb1571 Cr-Commit-Position: refs/heads/master@{#306842}

Patch Set 1 #

Total comments: 8

Patch Set 2 : unit tests added #

Total comments: 22

Patch Set 3 : refactoring, better action search #

Total comments: 8

Patch Set 4 : comments added #

Total comments: 8

Patch Set 5 : comments fix. changed check for boolean actions. #

Total comments: 14

Patch Set 6 : nits and clean up #

Total comments: 7

Patch Set 7 : minor fixes #

Total comments: 4

Patch Set 8 : minor fixes #

Patch Set 9 : actions.xml as a temporary file #

Total comments: 1

Patch Set 10 : comments for mock classes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+306 lines, -78 lines) Patch
M PRESUBMIT_test.py View 1 2 3 4 5 1 chunk +2 lines, -78 lines 0 comments Download
A PRESUBMIT_test_mocks.py View 1 2 3 4 5 6 7 8 9 1 chunk +109 lines, -0 lines 0 comments Download
A chrome/browser/resources/PRESUBMIT.py View 1 2 3 4 5 6 7 1 chunk +95 lines, -0 lines 0 comments Download
A chrome/browser/resources/PRESUBMIT_test.py View 1 2 3 4 5 6 7 8 1 chunk +100 lines, -0 lines 0 comments Download

Messages

Total messages: 38 (9 generated)
gayane -on leave until 09-2017
Please have a look at this initial draft.
6 years, 1 month ago (2014-11-11 19:38:15 UTC) #2
Alexei Svitkine (slow)
https://codereview.chromium.org/719463003/diff/1/PRESUBMIT.py File PRESUBMIT.py (left): https://codereview.chromium.org/719463003/diff/1/PRESUBMIT.py#oldcode11 PRESUBMIT.py:11: Nit: Revert changes to this file. https://codereview.chromium.org/719463003/diff/1/chrome/browser/resources/PRESUBMIT.py File chrome/browser/resources/PRESUBMIT.py ...
6 years, 1 month ago (2014-11-11 20:36:42 UTC) #3
gayane -on leave until 09-2017
Please review changes after added unittests and fixes for the minor issues. https://codereview.chromium.org/719463003/diff/1/PRESUBMIT.py File PRESUBMIT.py ...
6 years, 1 month ago (2014-11-12 22:00:23 UTC) #6
yao
https://codereview.chromium.org/719463003/diff/60001/chrome/browser/resources/PRESUBMIT.py File chrome/browser/resources/PRESUBMIT.py (right): https://codereview.chromium.org/719463003/diff/60001/chrome/browser/resources/PRESUBMIT.py#newcode4 chrome/browser/resources/PRESUBMIT.py:4: """Presubmit script for HTML files in . Finish the ...
6 years, 1 month ago (2014-11-12 22:23:22 UTC) #7
Alexei Svitkine (slow)
https://codereview.chromium.org/719463003/diff/60001/PRESUBMIT_test.py File PRESUBMIT_test.py (right): https://codereview.chromium.org/719463003/diff/60001/PRESUBMIT_test.py#newcode15 PRESUBMIT_test.py:15: from PRESUBMIT_test_mockobjects import MockFile, MockInputApi, MockOutputApi,\ Nit: I prefer ...
6 years, 1 month ago (2014-11-12 22:27:00 UTC) #8
gayane -on leave until 09-2017
A lot of new changes. - added different checks for boolean and non boolean actions. ...
6 years, 1 month ago (2014-11-15 00:09:09 UTC) #9
Alexei Svitkine (slow)
https://codereview.chromium.org/719463003/diff/80001/chrome/browser/resources/PRESUBMIT.py File chrome/browser/resources/PRESUBMIT.py (right): https://codereview.chromium.org/719463003/diff/80001/chrome/browser/resources/PRESUBMIT.py#newcode1 chrome/browser/resources/PRESUBMIT.py:1: # Copyright (c) 2014 The Chromium Authors. All rights ...
6 years, 1 month ago (2014-11-15 00:15:48 UTC) #10
gayane -on leave until 09-2017
Done. Comments seem to big now. If feel like I can add or delete something ...
6 years, 1 month ago (2014-11-15 00:50:33 UTC) #11
Alexei Svitkine (slow)
https://codereview.chromium.org/719463003/diff/100001/chrome/browser/resources/PRESUBMIT.py File chrome/browser/resources/PRESUBMIT.py (right): https://codereview.chromium.org/719463003/diff/100001/chrome/browser/resources/PRESUBMIT.py#newcode36 chrome/browser/resources/PRESUBMIT.py:36: action_name = match.group(2) Hmm, maybe name this metric_name rather ...
6 years, 1 month ago (2014-11-15 01:07:48 UTC) #12
gayane -on leave until 09-2017
https://codereview.chromium.org/719463003/diff/100001/chrome/browser/resources/PRESUBMIT.py File chrome/browser/resources/PRESUBMIT.py (right): https://codereview.chromium.org/719463003/diff/100001/chrome/browser/resources/PRESUBMIT.py#newcode36 chrome/browser/resources/PRESUBMIT.py:36: action_name = match.group(2) On 2014/11/15 01:07:48, Alexei Svitkine wrote: ...
6 years, 1 month ago (2014-11-18 18:18:57 UTC) #13
Alexei Svitkine (slow)
LGTM, +maruel to review the presubmit bits
6 years, 1 month ago (2014-11-18 18:31:09 UTC) #15
M-A Ruel
https://codereview.chromium.org/719463003/diff/120001/PRESUBMIT_test.py File PRESUBMIT_test.py (right): https://codereview.chromium.org/719463003/diff/120001/PRESUBMIT_test.py#newcode15 PRESUBMIT_test.py:15: from PRESUBMIT_test_mocks import MockFile, MockInputApi, MockOutputApi,\ use multiple from ...
6 years ago (2014-11-26 18:15:48 UTC) #16
gayane -on leave until 09-2017
Done. https://codereview.chromium.org/719463003/diff/120001/PRESUBMIT_test.py File PRESUBMIT_test.py (right): https://codereview.chromium.org/719463003/diff/120001/PRESUBMIT_test.py#newcode15 PRESUBMIT_test.py:15: from PRESUBMIT_test_mocks import MockFile, MockInputApi, MockOutputApi,\ On 2014/11/26 ...
6 years ago (2014-11-26 20:28:46 UTC) #17
M-A Ruel
https://codereview.chromium.org/719463003/diff/140001/chrome/browser/resources/PRESUBMIT.py File chrome/browser/resources/PRESUBMIT.py (right): https://codereview.chromium.org/719463003/diff/140001/chrome/browser/resources/PRESUBMIT.py#newcode87 chrome/browser/resources/PRESUBMIT.py:87: any([input_api.re.search(type_re, match.group(i)) for i in (1, 3)])) remove [], ...
6 years ago (2014-11-26 20:43:27 UTC) #18
gayane -on leave until 09-2017
https://codereview.chromium.org/719463003/diff/140001/chrome/browser/resources/PRESUBMIT.py File chrome/browser/resources/PRESUBMIT.py (right): https://codereview.chromium.org/719463003/diff/140001/chrome/browser/resources/PRESUBMIT.py#newcode87 chrome/browser/resources/PRESUBMIT.py:87: any([input_api.re.search(type_re, match.group(i)) for i in (1, 3)])) On 2014/11/26 ...
6 years ago (2014-11-26 21:57:28 UTC) #19
M-A Ruel
lgtm https://codereview.chromium.org/719463003/diff/140001/chrome/browser/resources/PRESUBMIT_test.py File chrome/browser/resources/PRESUBMIT_test.py (right): https://codereview.chromium.org/719463003/diff/140001/chrome/browser/resources/PRESUBMIT_test.py#newcode11 chrome/browser/resources/PRESUBMIT_test.py:11: sys.path.append(os.path.abspath(os.path.dirname(__file__) + '/../../../')) On 2014/11/26 21:57:28, gayane wrote: ...
6 years ago (2014-11-28 17:34:25 UTC) #20
gayane -on leave until 09-2017
https://codereview.chromium.org/719463003/diff/160001/chrome/browser/resources/PRESUBMIT.py File chrome/browser/resources/PRESUBMIT.py (right): https://codereview.chromium.org/719463003/diff/160001/chrome/browser/resources/PRESUBMIT.py#newcode21 chrome/browser/resources/PRESUBMIT.py:21: file_filter = lambda f: f.LocalPath().endswith(('.html')) On 2014/11/28 17:34:24, M-A ...
6 years ago (2014-11-28 20:43:14 UTC) #21
gayane -on leave until 09-2017
nkostylev@chromium.org: Please owner's review for changes in chrome/browser/resources/* phajdan.jr@chromium.org: Please owner's review for PRESUBMIT_test.py PRESUBMIT_test_mocks.py ...
6 years ago (2014-11-28 20:46:04 UTC) #23
Paweł Hajdan Jr.
+brettw for base/test/data I think we historically avoided adding more things there unless necessary. Brett, ...
6 years ago (2014-12-01 11:54:52 UTC) #25
brettw
It would be better if this file was by the test rather than in a ...
6 years ago (2014-12-01 20:32:23 UTC) #26
gayane -on leave until 09-2017
Removed actions.xml file. Now creating it as a temporary file.
6 years ago (2014-12-02 16:58:32 UTC) #28
Paweł Hajdan Jr.
LGTM
6 years ago (2014-12-03 10:51:17 UTC) #29
gayane -on leave until 09-2017
On 2014/12/03 10:51:17, Paweł Hajdan Jr. wrote: > LGTM brettw@ do you think you can ...
6 years ago (2014-12-03 16:29:56 UTC) #30
brettw
LGTM with comment. https://codereview.chromium.org/719463003/diff/200001/PRESUBMIT_test_mocks.py File PRESUBMIT_test_mocks.py (right): https://codereview.chromium.org/719463003/diff/200001/PRESUBMIT_test_mocks.py#newcode12 PRESUBMIT_test_mocks.py:12: class MockInputApi(object): Can these classes have ...
6 years ago (2014-12-03 19:28:02 UTC) #31
gayane -on leave until 09-2017
On 2014/12/03 19:28:02, brettw wrote: > LGTM with comment. > > https://codereview.chromium.org/719463003/diff/200001/PRESUBMIT_test_mocks.py > File PRESUBMIT_test_mocks.py ...
6 years ago (2014-12-03 20:55:57 UTC) #33
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/719463003/240001
6 years ago (2014-12-04 16:30:27 UTC) #35
commit-bot: I haz the power
Committed patchset #10 (id:240001)
6 years ago (2014-12-04 17:09:59 UTC) #36
commit-bot: I haz the power
Patchset 10 (id:??) landed as https://crrev.com/3dff8c2676a1585b5945a8671f97d0b1e2eb1571 Cr-Commit-Position: refs/heads/master@{#306842}
6 years ago (2014-12-04 17:11:25 UTC) #37
michaelpg
6 years ago (2014-12-05 04:29:19 UTC) #38
Message was sent while issue was closed.
On 2014/12/04 17:11:25, I haz the power (commit-bot) wrote:
> Patchset 10 (id:??) landed as
> https://crrev.com/3dff8c2676a1585b5945a8671f97d0b1e2eb1571
> Cr-Commit-Position: refs/heads/master@{#306842}

This is preventing me from commiting a change that seems valid, or uploading a
change that seems valid.

https://codereview.chromium.org/747883002/

Powered by Google App Engine
This is Rietveld 408576698