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

Issue 14064011: Add url/ to banned_directories in clang style checker. (Closed)

Created:
7 years, 8 months ago by tfarina
Modified:
7 years, 8 months ago
Reviewers:
Nico, brettw
CC:
chromium-reviews, brettw
Visibility:
Public.

Description

Add url/ to banned_directories in clang style checker. BUG=229660 R=thakis@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=194481

Patch Set 1 #

Patch Set 2 : add TODO #

Total comments: 1

Patch Set 3 : behind a flag #

Total comments: 1

Patch Set 4 : fix chrome-plugins #

Total comments: 2

Patch Set 5 : make it private #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -7 lines) Patch
M tools/clang/plugins/ChromeClassTester.h View 1 2 4 2 chunks +5 lines, -1 line 0 comments Download
M tools/clang/plugins/ChromeClassTester.cpp View 1 2 4 2 chunks +7 lines, -2 lines 0 comments Download
M tools/clang/plugins/FindBadConstructs.cpp View 1 2 3 5 chunks +11 lines, -4 lines 0 comments Download

Messages

Total messages: 14 (0 generated)
tfarina
7 years, 8 months ago (2013-04-15 22:11:38 UTC) #1
brettw
We should add this back eventually when the problems are fixed.
7 years, 8 months ago (2013-04-15 23:17:32 UTC) #2
tfarina
On Mon, Apr 15, 2013 at 8:17 PM, <brettw@chromium.org> wrote: > We should add this ...
7 years, 8 months ago (2013-04-16 00:06:21 UTC) #3
tfarina
Added the TODO. Nico, please take another look. Thanks!
7 years, 8 months ago (2013-04-16 00:09:31 UTC) #4
Nico
lgtm either way. I was about to do a roll, but I suppose I'll wait ...
7 years, 8 months ago (2013-04-16 01:51:49 UTC) #5
tfarina
Nico, could you review it again and see if patch set 3 implements it correctly ...
7 years, 8 months ago (2013-04-16 03:05:50 UTC) #6
Nico
Looks basically right, but one question: https://codereview.chromium.org/14064011/diff/8005/tools/clang/plugins/FindBadConstructs.cpp File tools/clang/plugins/FindBadConstructs.cpp (right): https://codereview.chromium.org/14064011/diff/8005/tools/clang/plugins/FindBadConstructs.cpp#newcode83 tools/clang/plugins/FindBadConstructs.cpp:83: check_url_directory_(check_url_directory) { Do ...
7 years, 8 months ago (2013-04-16 15:31:11 UTC) #7
tfarina
On 2013/04/16 15:31:11, Nico wrote: > Looks basically right, but one question: > > https://codereview.chromium.org/14064011/diff/8005/tools/clang/plugins/FindBadConstructs.cpp ...
7 years, 8 months ago (2013-04-16 16:11:27 UTC) #8
Nico
On Tue, Apr 16, 2013 at 9:11 AM, <tfarina@chromium.org> wrote: > On 2013/04/16 15:31:11, Nico ...
7 years, 8 months ago (2013-04-16 17:21:44 UTC) #9
tfarina
Nico, could you review patch set 4 again? You were right, there was two errors. ...
7 years, 8 months ago (2013-04-16 22:03:54 UTC) #10
Nico
https://codereview.chromium.org/14064011/diff/19001/tools/clang/plugins/ChromeClassTester.h File tools/clang/plugins/ChromeClassTester.h (right): https://codereview.chromium.org/14064011/diff/19001/tools/clang/plugins/ChromeClassTester.h#newcode49 tools/clang/plugins/ChromeClassTester.h:49: bool check_url_directory_; I think this can stay private. You ...
7 years, 8 months ago (2013-04-16 22:16:56 UTC) #11
tfarina
https://codereview.chromium.org/14064011/diff/19001/tools/clang/plugins/ChromeClassTester.h File tools/clang/plugins/ChromeClassTester.h (right): https://codereview.chromium.org/14064011/diff/19001/tools/clang/plugins/ChromeClassTester.h#newcode49 tools/clang/plugins/ChromeClassTester.h:49: bool check_url_directory_; On 2013/04/16 22:16:56, Nico wrote: > I ...
7 years, 8 months ago (2013-04-16 22:23:24 UTC) #12
Nico
lgtm
7 years, 8 months ago (2013-04-16 22:35:56 UTC) #13
tfarina
7 years, 8 months ago (2013-04-16 23:22:34 UTC) #14
Message was sent while issue was closed.
Committed patchset #5 manually as r194481 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698