[MD settings] moving settings strings to a separate grd file

[MD settings] moving settings strings to a separate grd file

BUG=425627
Committed: https://crrev.com/7b8e80f197eb54799ab65b3c6e554e5d50b23351
Cr-Commit-Position: refs/heads/master@{#351888}

[newt (away), 2015-09-30 20:11:42]

newt@chromium.org changed reviewers:
+ newt@chromium.org

[newt (away), 2015-09-30 20:11:43]

looks generally good. There's one unneeded gyp file (probably just development
cruft), and of course you'll need GN changes as well.

https://codereview.chromium.org/1377213003/diff/1/chrome/app/settings_strings...
File chrome/app/settings_strings.gyp (right):

https://codereview.chromium.org/1377213003/diff/1/chrome/app/settings_strings...
chrome/app/settings_strings.gyp:1: # Copyright 2014 The Chromium Authors. All
rights reserved.
shouldn't need this file

https://codereview.chromium.org/1377213003/diff/1/tools/gritsettings/translat...
File tools/gritsettings/translation_expectations.pyl (right):

https://codereview.chromium.org/1377213003/diff/1/tools/gritsettings/translat...
tools/gritsettings/translation_expectations.pyl:66:
"chrome/app/settings_strings.grd": "Work in progress; to be localized later in
development",
might be helpful to add an approximate date/milestone here

[Dan Beam, 2015-09-30 21:29:46]

dbeam@chromium.org changed reviewers:
+ dbeam@chromium.org

[Dan Beam, 2015-09-30 21:29:46]

why do we need all these localized versions until we're translating?

[newt (away), 2015-09-30 21:40:12]

Right. You don't need these xtb files. Just delete the files and remove the
<translations> element and the xtb <output> elements from the grd file.

[newt (away), 2015-09-30 21:42:10]

https://chromiumcodereview.appspot.com/1377213003/diff/20001/chrome/app/gener...
File chrome/app/generated_resources.grd (left):

https://chromiumcodereview.appspot.com/1377213003/diff/20001/chrome/app/gener...
chrome/app/generated_resources.grd:195: -      <part
file="settings_strings.grdp" />
Note that these strings will no longer be translated. So if anyone modifies an
existing string in settings_strings.grdp, the next release will be missing
translations for that string.

A safer alternative is to add new strings in a new grdp file, and to keep the
existing strings running through the translation process.

[newt (away), 2015-09-30 21:43:18]

(Adding translateable="false" to each new string is also still an option.)

[dschuyler, 2015-09-30 21:43:44]

On 2015/09/30 21:29:46, Dan Beam wrote:
> why do we need all these localized versions until we're translating?

I'm hoping we don't.  Newton offered to take a peek at it, so I put
up the CL for him to look at.  I'm still looking into it as well.

[dschuyler, 2015-09-30 22:37:28]

https://chromiumcodereview.appspot.com/1377213003/diff/1/chrome/app/settings_...
File chrome/app/settings_strings.gyp (right):

https://chromiumcodereview.appspot.com/1377213003/diff/1/chrome/app/settings_...
chrome/app/settings_strings.gyp:1: # Copyright 2014 The Chromium Authors. All
rights reserved.
On 2015/09/30 20:11:43, newt wrote:
> shouldn't need this file

Done.

https://chromiumcodereview.appspot.com/1377213003/diff/1/tools/gritsettings/t...
File tools/gritsettings/translation_expectations.pyl (right):

https://chromiumcodereview.appspot.com/1377213003/diff/1/tools/gritsettings/t...
tools/gritsettings/translation_expectations.pyl:66:
"chrome/app/settings_strings.grd": "Work in progress; to be localized later in
development",
On 2015/09/30 20:11:43, newt wrote:
> might be helpful to add an approximate date/milestone here

Done.

https://chromiumcodereview.appspot.com/1377213003/diff/20001/chrome/app/gener...
File chrome/app/generated_resources.grd (left):

https://chromiumcodereview.appspot.com/1377213003/diff/20001/chrome/app/gener...
chrome/app/generated_resources.grd:195: -      <part
file="settings_strings.grdp" />
On 2015/09/30 21:42:10, newt wrote:
> Note that these strings will no longer be translated. So if anyone modifies an
> existing string in settings_strings.grdp, the next release will be missing
> translations for that string.

That's what we're going for :)

> 
> A safer alternative is to add new strings in a new grdp file, and to keep the
> existing strings running through the translation process.

We kind of did that, but the grdp file already got created.  The
settings_strings.grdp files is the new file :)  The existing strings are in
other, older files (that will be culled at a later date).

[newt (away), 2015-09-30 22:44:22]

> > 
> > A safer alternative is to add new strings in a new grdp file, and to keep
the
> > existing strings running through the translation process.
> 
> We kind of did that, but the grdp file already got created.  The
> settings_strings.grdp files is the new file :)  The existing strings are in
> other, older files (that will be culled at a later date).

Ah, then this looks correct. Though all the strings in settings_strings.grdp
have already been translated, so it would have been more useful to do this when
the file was first added.

[newt (away), 2015-09-30 22:46:20]

lgtm

[dschuyler, 2015-10-01 01:21:12]

Thanks Newton :)

Dan, I think it's ready for a review (the xtb files have been cleaned up and
such).

[Dan Beam, 2015-10-01 01:26:07]

https://chromiumcodereview.appspot.com/1377213003/diff/100001/chrome/app/BUIL...
File chrome/app/BUILD.gn (right):

https://chromiumcodereview.appspot.com/1377213003/diff/100001/chrome/app/BUIL...
chrome/app/BUILD.gn:307: "settings_strings_am.pak",
then why are these still here?

[dschuyler, 2015-10-01 18:50:47]

https://chromiumcodereview.appspot.com/1377213003/diff/100001/chrome/app/BUIL...
File chrome/app/BUILD.gn (right):

https://chromiumcodereview.appspot.com/1377213003/diff/100001/chrome/app/BUIL...
chrome/app/BUILD.gn:307: "settings_strings_am.pak",
On 2015/10/01 01:26:06, Dan Beam wrote:
> then why are these still here?

These are the small pass through files.  We still get the strings from a single
English.pak.  I think we can remove them if we treat the stings as general
resources instead of strings.

[Dan Beam, 2015-10-01 20:09:13]

lgtm

[dschuyler, 2015-10-01 20:50:49]

The CQ bit was checked by dschuyler@chromium.org

[dschuyler, 2015-10-01 20:50:49]

The patchset sent to the CQ was uploaded after l-g-t-m from newt@chromium.org
Link to the patchset:
https://chromiumcodereview.appspot.com/1377213003/#ps120001 (title: "removing
language pak files for settings strings")

[commit-bot: I haz the power, 2015-10-01 20:51:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1377213003/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1377213003/120001

[commit-bot: I haz the power, 2015-10-01 21:01:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-01 21:01:25]

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...)

[Lei Zhang, 2015-10-01 21:16:29]

thestig@chromium.org changed reviewers:
+ thestig@chromium.org

[Lei Zhang, 2015-10-01 21:16:29]

lgtm

[dschuyler, 2015-10-01 21:16:51]

The CQ bit was checked by dschuyler@chromium.org

[commit-bot: I haz the power, 2015-10-01 21:18:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1377213003/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1377213003/120001

[commit-bot: I haz the power, 2015-10-01 21:24:32]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-10-01 21:25:17]

Patchset 7 (id:??) landed as
https://crrev.com/7b8e80f197eb54799ab65b3c6e554e5d50b23351
Cr-Commit-Position: refs/heads/master@{#351888}