Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2572773002:
Add page info strings for offline pages. (Closed)
DescriptionAdd page info strings for offline pages.
BUG=671137
Committed: https://crrev.com/7c7f83aa87578c22ae7ae8f6987928f367b305d9
Cr-Commit-Position: refs/heads/master@{#438796}
Patch Set 1 #Patch Set 2 : remove space #
Total comments: 3
Patch Set 3 : move strings #Messages
Total messages: 19 (8 generated)
olivierrobin@chromium.org changed reviewers: + jif@chromium.org, rohitrao@chromium.org
rohitrao@chromium.org changed reviewers: + sdefresne@chromium.org
Strings themselves look fine, but I'm not an OWNER of this file. If the strings are ios-only, should they live in ios_strings.grd instead of in components? https://codereview.chromium.org/2572773002/diff/20001/components/pageinfo_str... File components/pageinfo_strings.grdp (right): https://codereview.chromium.org/2572773002/diff/20001/components/pageinfo_str... components/pageinfo_strings.grdp:74: <if expr="is_ios"> Should these live behind is_ios, or should they just be cross-platform strings?
jif@google.com changed reviewers: + jif@google.com
lgtm https://codereview.chromium.org/2572773002/diff/20001/components/pageinfo_str... File components/pageinfo_strings.grdp (right): https://codereview.chromium.org/2572773002/diff/20001/components/pageinfo_str... components/pageinfo_strings.grdp:74: <if expr="is_ios"> On 2016/12/13 14:19:40, rohitrao wrote: > Should these live behind is_ios, or should they just be cross-platform strings? The strings are not used by any other platform, so behind is_ios.
https://codereview.chromium.org/2572773002/diff/20001/components/pageinfo_str... File components/pageinfo_strings.grdp (right): https://codereview.chromium.org/2572773002/diff/20001/components/pageinfo_str... components/pageinfo_strings.grdp:74: <if expr="is_ios"> On 2016/12/13 14:29:14, jif-google wrote: > On 2016/12/13 14:19:40, rohitrao wrote: > > Should these live behind is_ios, or should they just be cross-platform > strings? > > The strings are not used by any other platform, so behind is_ios. The strings are never used from the components, so I moved them in ios_strings.
Thanks. PTAL
lgtm
olivierrobin@chromium.org changed reviewers: + noyau@chromium.org
+noyau for owner.
The CQ bit was checked by noyau@chromium.org
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481795314248120,
"parent_rev": "bc914bf435f3f488b7862f9f9772365d0430ec0e", "commit_rev":
"7348614f6a452ee0b50073269d3f57433f7ed828"}
Message was sent while issue was closed.
Description was changed from ========== Add page info strings for offline pages. BUG=671137 ========== to ========== Add page info strings for offline pages. BUG=671137 Review-Url: https://codereview.chromium.org/2572773002 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Add page info strings for offline pages. BUG=671137 Review-Url: https://codereview.chromium.org/2572773002 ========== to ========== Add page info strings for offline pages. BUG=671137 Committed: https://crrev.com/7c7f83aa87578c22ae7ae8f6987928f367b305d9 Cr-Commit-Position: refs/heads/master@{#438796} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/7c7f83aa87578c22ae7ae8f6987928f367b305d9 Cr-Commit-Position: refs/heads/master@{#438796} |
