Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
11419316:
webui: Replace wstring with string16 for one variable. (Closed)
|
Created:
8 years ago by exxi Modified:
8 years ago CC:
chromium-reviews Base URL:
http://src.chromium.org/svn/trunk/src/ Visibility:
Public. |
Descriptionwebui: Replace wstring with string16 for one variable.
BUG=23581
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=173623
Patch Set 1 #Patch Set 2 : #
Total comments: 4
Patch Set 3 : #Patch Set 4 : #Messages
Total messages: 21 (0 generated)
LGTM.
On 2012/12/04 20:39:10, Tom Sepez wrote: > LGTM. This is my first change, could you commit it for me?
I used "reply" earlier which should have been "Publish+Mail Comments". This is my first change, could you commit it for me or is there anything else I need to do previously?
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/m.s.bednorz@gmail.com/11419316/1
Presubmit check for 11419316-1 failed and returned exit status 1. Running presubmit commit checks ... ** Presubmit Warnings ** m.s.bednorz@gmail.com is not in AUTHORS file. If you are a new contributor, please visit http://www.chromium.org/developers/contributing-code and read the "Legal" section If you are a chromite, verify the contributor signed the CLA. ** Presubmit ERRORS ** Missing LGTM from an OWNER for files in these directories: chrome/browser/ui/webui Presubmit checks took 4.0s to calculate.
Reviewer from OWNERS file added.
https://codereview.chromium.org/11419316/diff/10001/chrome/browser/ui/webui/a... File chrome/browser/ui/webui/about_ui.cc (right): https://codereview.chromium.org/11419316/diff/10001/chrome/browser/ui/webui/a... chrome/browser/ui/webui/about_ui.cc:893: if (log_string.length() > 0) !empty https://codereview.chromium.org/11419316/diff/10001/chrome/browser/ui/webui/a... chrome/browser/ui/webui/about_ui.cc:895: log_string.append(browser_processes[index].name); This feels like more code than it needs to be. How about: log_string += browser_processes[index].name + ASCIIToUTF16(", ") + ... https://codereview.chromium.org/11419316/diff/10001/chrome/browser/ui/webui/a... chrome/browser/ui/webui/about_ui.cc:903: if (log_string.length() > 0) !empty
New snapshot uploaded. https://codereview.chromium.org/11419316/diff/10001/chrome/browser/ui/webui/a... File chrome/browser/ui/webui/about_ui.cc (right): https://codereview.chromium.org/11419316/diff/10001/chrome/browser/ui/webui/a... chrome/browser/ui/webui/about_ui.cc:903: if (log_string.length() > 0) On 2012/12/06 21:05:01, sky wrote: > !empty Done.
LGTM
Could one of the reviewers start a new trybot run or commit the changes? Sorry for all the messages for such a small change.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/m.s.bednorz@gmail.com/11419316/14001
Retried try job too often on win_rel for step(s) interactive_ui_tests
Should I try the changes on a windows machine or are the win_rel failures unrelated to my changes? I can't figure out why they would break, especially because the first Patch had no failures on win_rel and the third Patch is not that different.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/m.s.bednorz@gmail.com/11419316/14001
Sorry for I got bad news for ya. Compile failed with a clobber build on win_rel. http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu... Your code is likely broken or HEAD is junk. Please ensure your code is not broken then alert the build sheriffs. Look at the try server FAQ for more details.
Needed to update my AUTHORS file, could you run the trybots again?
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/m.s.bednorz@gmail.com/11419316/26001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/m.s.bednorz@gmail.com/11419316/26001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/m.s.bednorz@gmail.com/11419316/26001
Message was sent while issue was closed.
Change committed as 173623 |
