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

Issue 10068025: views: Initialize message_box_title_ in the initalizer list. (Closed)

Created:
8 years, 8 months ago by tfarina
Modified:
8 years, 8 months ago
Reviewers:
Peter Kasting
CC:
chromium-reviews
Visibility:
Public.

Description

views: Initialize message_box_title_ in the initalizer list. R=pkasting@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=132247

Patch Set 1 #

Patch Set 2 : #

Total comments: 2

Patch Set 3 : peter review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -14 lines) Patch
M chrome/browser/ui/views/simple_message_box_views.h View 1 2 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/ui/views/simple_message_box_views.cc View 1 2 3 chunks +8 lines, -10 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
tfarina
8 years, 8 months ago (2012-04-12 20:16:07 UTC) #1
Peter Kasting
8 years, 8 months ago (2012-04-13 09:12:32 UTC) #2
LGTM

http://codereview.chromium.org/10068025/diff/4/chrome/browser/ui/views/simple...
File chrome/browser/ui/views/simple_message_box_views.cc (right):

http://codereview.chromium.org/10068025/diff/4/chrome/browser/ui/views/simple...
chrome/browser/ui/views/simple_message_box_views.cc:135: message_box_view_ = new
views::MessageBoxView(
Nit: Why not init this in the initializer list too?

http://codereview.chromium.org/10068025/diff/4/chrome/browser/ui/views/simple...
File chrome/browser/ui/views/simple_message_box_views.h (right):

http://codereview.chromium.org/10068025/diff/4/chrome/browser/ui/views/simple...
chrome/browser/ui/views/simple_message_box_views.h:75: DispositionType
disposition_type_;
Nit: While it's not wrong, I don't think the |disposition_type_| name is clearer
or more distinct -- if anything I think it's a tiny bit less good. 
(|dialog_type_| is fine.)

Powered by Google App Engine
This is Rietveld 408576698