Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1118443002:
Replacing const by value return values into const by ref in chrome/browser/ui/ (Closed)
|
Created:
5 years, 7 months ago by payal.pandey Modified:
5 years, 7 months ago CC:
chromium-reviews, skanuj+watch_chromium.org, melevin+watch_chromium.org, dhollowa+watch_chromium.org, dougw+watch_chromium.org, donnd+watch_chromium.org, jfweitz+watch_chromium.org, David Black, samarth+watch_chromium.org, kmadhusu+watch_chromium.org, Jered Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionReplacing const by value return values into const by ref in chrome/browser/ui/
This CL turns const by value return values into const by ref
BUG=393155
Committed: https://crrev.com/0dda3d29e6d5544ea11f5ae2683b2fe11270f523
Cr-Commit-Position: refs/heads/master@{#327569}
Patch Set 1 #
Messages
Total messages: 24 (6 generated)
payal.pandey@samsung.com changed reviewers: + jochen@chromium.org
Please have a look, Thanks.
payal.pandey@samsung.com changed reviewers: + avi@chromium.org, sky@chromium.org
Please review the CL, Thanks
On 2015/04/29 18:15:55, payal.pandey wrote: > Please review the CL, Thanks Your description is not helpful and ungrammatical. Remove it. Code-wise, lgtm
On 2015/04/29 18:19:24, Avi wrote: > On 2015/04/29 18:15:55, payal.pandey wrote: > > Please review the CL, Thanks > > Your description is not helpful and ungrammatical. Remove it. > > Code-wise, lgtm @Avi : Updated the description, Thanks for lgtm.
The CQ bit was checked by payal.pandey@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1118443002/1
On 2015/04/29 18:25:40, payal.pandey wrote: > On 2015/04/29 18:19:24, Avi wrote: > > On 2015/04/29 18:15:55, payal.pandey wrote: > > > Please review the CL, Thanks > > > > Your description is not helpful and ungrammatical. Remove it. > > > > Code-wise, lgtm > > @Avi : Updated the description, Thanks for lgtm. What is the reference to extensions/common/?
On 2015/04/29 18:26:47, Anuj wrote: > On 2015/04/29 18:25:40, payal.pandey wrote: > > On 2015/04/29 18:19:24, Avi wrote: > > > On 2015/04/29 18:15:55, payal.pandey wrote: > > > > Please review the CL, Thanks > > > > > > Your description is not helpful and ungrammatical. Remove it. > > > > > > Code-wise, lgtm > > > > @Avi : Updated the description, Thanks for lgtm. > > What is the reference to extensions/common/?
On 2015/04/29 18:33:47, Avi wrote: > On 2015/04/29 18:26:47, Anuj wrote: > > On 2015/04/29 18:25:40, payal.pandey wrote: > > > On 2015/04/29 18:19:24, Avi wrote: > > > > On 2015/04/29 18:15:55, payal.pandey wrote: > > > > > Please review the CL, Thanks > > > > > > > > Your description is not helpful and ungrammatical. Remove it. > > > > > > > > Code-wise, lgtm > > > > > > @Avi : Updated the description, Thanks for lgtm. > > > > What is the reference to extensions/common/? I asked for its removal. Explaining the benefits of using const ref isn't helpful.
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
@Avi: Is description is fine now? Or do i need to remove completely ?
On 2015/04/29 18:42:49, payal.pandey wrote: > @Avi: Is description is fine now? > Or do i need to remove completely ? Drop the second line. Also, I'm not a valid reviewer for that file.
On 2015/04/29 18:44:43, Avi wrote: > On 2015/04/29 18:42:49, payal.pandey wrote: > > @Avi: Is description is fine now? > > Or do i need to remove completely ? > > Drop the second line. > > Also, I'm not a valid reviewer for that file. Okay, and who would be valid reviewer for this file.
payal.pandey@samsung.com changed reviewers: + msw@chromium.org
On 2015/04/29 18:48:43, payal.pandey wrote: > On 2015/04/29 18:44:43, Avi wrote: > > On 2015/04/29 18:42:49, payal.pandey wrote: > > > @Avi: Is description is fine now? > > > Or do i need to remove completely ? > > > > Drop the second line. > > > > Also, I'm not a valid reviewer for that file. > > Okay, and who would be valid reviewer for this file. To find a reviewer: - Locate the file in cs.chromium.org (https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...) - Use "View in" > "Git revision log" from right top (https://chromium.googlesource.com/chromium/src/+log/master/chrome/browser/ui/...) - Find a non-trivial CL. In this file, fortunately most recent change is non-trivial. (https://chromium.googlesource.com/chromium/src/+/880d328f729c6986b2502467a477...) - Follow the review url in change description. (https://codereview.chromium.org/930853005) - Find the reviewers who approved that CL-> kmadhusu@chromium.org
lgtm
The CQ bit was checked by payal.pandey@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1118443002/1
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/0dda3d29e6d5544ea11f5ae2683b2fe11270f523 Cr-Commit-Position: refs/heads/master@{#327569}
Message was sent while issue was closed.
On 2015/04/29 20:02:19, Anuj wrote: > On 2015/04/29 18:48:43, payal.pandey wrote: > > On 2015/04/29 18:44:43, Avi wrote: > > > On 2015/04/29 18:42:49, payal.pandey wrote: > > > > @Avi: Is description is fine now? > > > > Or do i need to remove completely ? > > > > > > Drop the second line. > > > > > > Also, I'm not a valid reviewer for that file. > > > > Okay, and who would be valid reviewer for this file. > > To find a reviewer: > - Locate the file in http://cs.chromium.org > (https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...) > - Use "View in" > "Git revision log" from right top > (https://chromium.googlesource.com/chromium/src/+log/master/chrome/browser/ui/...) > - Find a non-trivial CL. In this file, fortunately most recent change is > non-trivial. > (https://chromium.googlesource.com/chromium/src/+/880d328f729c6986b2502467a477...) > - Follow the review url in change description. > (https://codereview.chromium.org/930853005) > - Find the reviewers who approved that CL-> mailto:kmadhusu@chromium.org Actually, he found a reviewer, but he needed an OWNER. https://www.chromium.org/developers/owners-files |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
