MD Settings: About page, localize remaining strings.

MD Settings: About page, localize remaining strings.

BUG=627929
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/8379f51dc3a4b447bbce3f479aece354214914bc
Cr-Commit-Position: refs/heads/master@{#405354}

[dpapad, 2016-07-13 21:05:09]

Description was changed from

==========
MD Settings: About page, localize remaining strings.

BUG=627929
==========

to

==========
MD Settings: About page, localize remaining strings.

BUG=627929
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[dpapad, 2016-07-13 21:12:02]

Patchset #1 (id:1) has been deleted

[dpapad, 2016-07-13 21:12:57]

The CQ bit was checked by dpapad@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-13 21:13:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-13 21:13:51]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[dpapad, 2016-07-13 21:17:46]

dpapad@chromium.org changed reviewers:
+ tommycli@chromium.org

[tommycli, 2016-07-13 21:54:06]

LGTM sans below issues, and assuming you've tested this manually to make sure
all the strings show up correctly.

https://codereview.chromium.org/2147063002/diff/20001/chrome/app/settings_str...
File chrome/app/settings_strings.grdp (right):

https://codereview.chromium.org/2147063002/diff/20001/chrome/app/settings_str...
chrome/app/settings_strings.grdp:30: <message
name="IDS_SETTINGS_ABOUT_PAGE_FIRMWARE" desc="Label used for Chrome OS bios
firmware version.">
nit: Isn't it always BIOS all caps?

https://codereview.chromium.org/2147063002/diff/20001/chrome/app/settings_str...
chrome/app/settings_strings.grdp:33: <message
name="IDS_SETTINGS_ABOUT_PAGE_PLATFORM" desc="Label for Platform">
Maybe expand desc. to "Label for Platform (i.e. Linux, Mac, Windows)" assuming
this means OS.

https://codereview.chromium.org/2147063002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/about_page/detailed_build_info.html
(right):

https://codereview.chromium.org/2147063002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/about_page/detailed_build_info.html:56:
<div>Blink</div>
Needs $i18n tag here

[dpapad, 2016-07-13 22:06:32]

https://codereview.chromium.org/2147063002/diff/20001/chrome/app/settings_str...
File chrome/app/settings_strings.grdp (right):

https://codereview.chromium.org/2147063002/diff/20001/chrome/app/settings_str...
chrome/app/settings_strings.grdp:30: <message
name="IDS_SETTINGS_ABOUT_PAGE_FIRMWARE" desc="Label used for Chrome OS bios
firmware version.">
On 2016/07/13 at 21:54:05, tommycli wrote:
> nit: Isn't it always BIOS all caps?

Done (these descriptions were simply copied from old code,
https://cs.chromium.org/chromium/src/chrome/app/generated_resources.grd?type=...).

https://codereview.chromium.org/2147063002/diff/20001/chrome/app/settings_str...
chrome/app/settings_strings.grdp:33: <message
name="IDS_SETTINGS_ABOUT_PAGE_PLATFORM" desc="Label for Platform">
On 2016/07/13 at 21:54:05, tommycli wrote:
> Maybe expand desc. to "Label for Platform (i.e. Linux, Mac, Windows)" assuming
this means OS.

It does not mean linux/mac/windows. This code is only used in ChromeOS, and the
value looks similar to "Platform 8172.60.0 (Official Build) stable-channel
samus"

I could not think of a way to better describe that string, so I stuck with the
pre-existing description, see
https://cs.chromium.org/chromium/src/chrome/app/generated_resources.grd?q=%22....

https://codereview.chromium.org/2147063002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/about_page/detailed_build_info.html
(right):

https://codereview.chromium.org/2147063002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/about_page/detailed_build_info.html:56:
<div>Blink</div>
On 2016/07/13 at 21:54:05, tommycli wrote:
> Needs $i18n tag here

Don't see a value in localizing this. The old Options was seemingly localizing
this, but the comment says "should not require translation", see
https://cs.chromium.org/chromium/src/chrome/app/generated_resources.grd?q=IDS....

[tommycli, 2016-07-13 22:18:43]

okay thanks

https://codereview.chromium.org/2147063002/diff/20001/chrome/app/settings_str...
File chrome/app/settings_strings.grdp (right):

https://codereview.chromium.org/2147063002/diff/20001/chrome/app/settings_str...
chrome/app/settings_strings.grdp:33: <message
name="IDS_SETTINGS_ABOUT_PAGE_PLATFORM" desc="Label for Platform">
On 2016/07/13 22:06:32, dpapad wrote:
> On 2016/07/13 at 21:54:05, tommycli wrote:
> > Maybe expand desc. to "Label for Platform (i.e. Linux, Mac, Windows)"
assuming
> this means OS.
> 
> It does not mean linux/mac/windows. This code is only used in ChromeOS, and
the
> value looks similar to "Platform 8172.60.0 (Official Build) stable-channel
> samus"
> 
> I could not think of a way to better describe that string, so I stuck with the
> pre-existing description, see
>
https://cs.chromium.org/chromium/src/chrome/app/generated_resources.grd?q=%22....
>  

Oh I see. Kind of hard to describe I guess. At least it's no worse than before.

https://codereview.chromium.org/2147063002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/about_page/detailed_build_info.html
(right):

https://codereview.chromium.org/2147063002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/about_page/detailed_build_info.html:56:
<div>Blink</div>
On 2016/07/13 22:06:32, dpapad wrote:
> On 2016/07/13 at 21:54:05, tommycli wrote:
> > Needs $i18n tag here
> 
> Don't see a value in localizing this. The old Options was seemingly localizing
> this, but the comment says "should not require translation", see
>
https://cs.chromium.org/chromium/src/chrome/app/generated_resources.grd?q=IDS....

I see. Add a comment explaining "Blink should never be localized"?

[dpapad, 2016-07-13 22:31:13]

Thanks!

https://codereview.chromium.org/2147063002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/about_page/detailed_build_info.html
(right):

https://codereview.chromium.org/2147063002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/about_page/detailed_build_info.html:56:
<div>Blink</div>

> I see. Add a comment explaining "Blink should never be localized"?

Added comment.

[dpapad, 2016-07-13 22:33:58]

The CQ bit was checked by dpapad@chromium.org

[dpapad, 2016-07-13 22:33:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
tommycli@chromium.org
Link to the patchset: https://codereview.chromium.org/2147063002/#ps60001
(title: "Nit")

[commit-bot: I haz the power, 2016-07-13 22:36:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-13 23:59:13]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-07-13 23:59:32]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-14 00:01:03]

Description was changed from

==========
MD Settings: About page, localize remaining strings.

BUG=627929
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: About page, localize remaining strings.

BUG=627929
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/8379f51dc3a4b447bbce3f479aece354214914bc
Cr-Commit-Position: refs/heads/master@{#405354}
==========

[commit-bot: I haz the power, 2016-07-14 00:01:04]

Patchset 3 (id:??) landed as
https://crrev.com/8379f51dc3a4b447bbce3f479aece354214914bc
Cr-Commit-Position: refs/heads/master@{#405354}