Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1469003002:
Permissions channel filter should use IDS_PRODUCT_NAME (Closed)
|
Created:
5 years, 1 month ago by wmaslowski Modified:
5 years ago CC:
chromium-apps-reviews_chromium.org, chromium-reviews, extensions-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionPermissions channel filter should use IDS_PRODUCT_NAME
... instead of explicit 'Google Chrome'.
BUG=560262
Committed: https://crrev.com/8716788e85dca95dac035ca6bb408714ada246df
Cr-Commit-Position: refs/heads/master@{#365825}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Add missing parameter #
Total comments: 1
Patch Set 3 : Combine the string lines #
Total comments: 1
Patch Set 4 : fix includes #Messages
Total messages: 19 (6 generated)
wmaslowski@opera.com changed reviewers: + reillyg@chromium.org, rockot@chromium.org
https://codereview.chromium.org/1469003002/diff/1/chrome/common/extensions/fe... File chrome/common/extensions/features/chrome_channel_feature_filter.cc (right): https://codereview.chromium.org/1469003002/diff/1/chrome/common/extensions/fe... chrome/common/extensions/features/chrome_channel_feature_filter.cc:99: GetChannelName(channel_).c_str())); This format string requires the current channel name as well but that parameter has been removed.
Added missing parameter https://codereview.chromium.org/1469003002/diff/1/chrome/common/extensions/fe... File chrome/common/extensions/features/chrome_channel_feature_filter.cc (right): https://codereview.chromium.org/1469003002/diff/1/chrome/common/extensions/fe... chrome/common/extensions/features/chrome_channel_feature_filter.cc:99: GetChannelName(channel_).c_str())); On 2015/11/23 19:23:32, Reilly Grant wrote: > This format string requires the current channel name as well but that parameter > has been removed. whoops - seems like I commited it before correcting
lgtm with nit https://codereview.chromium.org/1469003002/diff/20001/chrome/common/extension... File chrome/common/extensions/features/chrome_channel_feature_filter.cc (right): https://codereview.chromium.org/1469003002/diff/20001/chrome/common/extension... chrome/common/extensions/features/chrome_channel_feature_filter.cc:96: "%s channel.", If it fits now (I think it does), you should merge this with the previous line.
Fix the style issue
lgtm with one nit https://codereview.chromium.org/1469003002/diff/40001/chrome/common/extension... File chrome/common/extensions/features/chrome_channel_feature_filter.cc (right): https://codereview.chromium.org/1469003002/diff/40001/chrome/common/extension... chrome/common/extensions/features/chrome_channel_feature_filter.cc:15: #include "grit/chromium_strings.h" chrome/grit/chromium_strings.h The compiler allows both but the full path is preferred.
fixed include problems
lgtm
The CQ bit was checked by reillyg@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1469003002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1469003002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by wmaslowski@opera.com
The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org Link to the patchset: https://codereview.chromium.org/1469003002/#ps60001 (title: "fix includes")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1469003002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1469003002/60001
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Permissions channel filter should use IDS_PRODUCT_NAME ... instead of explicit 'Google Chrome'. BUG=560262 ========== to ========== Permissions channel filter should use IDS_PRODUCT_NAME ... instead of explicit 'Google Chrome'. BUG=560262 Committed: https://crrev.com/8716788e85dca95dac035ca6bb408714ada246df Cr-Commit-Position: refs/heads/master@{#365825} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/8716788e85dca95dac035ca6bb408714ada246df Cr-Commit-Position: refs/heads/master@{#365825} |
