Add athena_strings.grd with athena specific strings

Add athena_strings.grd with athena specific strings

BUG=416209
TEST=None

Committed: https://crrev.com/e0b167731257fd81b27f26b8448f2980ee06d7b5
Cr-Commit-Position: refs/heads/master@{#296797}

[pkotwicz, 2014-09-20 02:05:20]

Patchset #1 (id:1) has been deleted

[pkotwicz, 2014-09-20 02:08:47]

Patchset #1 (id:20001) has been deleted

[pkotwicz, 2014-09-20 02:11:08]

pkotwicz@chromium.org changed reviewers:
+ tony@chromium.org

[pkotwicz, 2014-09-20 02:11:09]

Tony, can you please take a look?

[tony, 2014-09-20 03:06:58]

I don't work on Chromium these days.  Can you get someone else to review it and
if you need owners approval, I can provide a rubber stamp approval.  Oshima
would be a good reviewer.

As a side note, I would prefer a more descriptive change description (I don't
know what athena is, but maybe people more active with the project do).

[pkotwicz, 2014-09-22 05:02:20]

Patchset #2 (id:60001) has been deleted

[pkotwicz, 2014-09-22 15:16:25]

pkotwicz@chromium.org changed reviewers:
+ jamescook@chormium.org

[pkotwicz, 2014-09-22 15:16:26]

James, can you please take a look? I think you did something similar for
extensions recently

[pkotwicz, 2014-09-24 02:07:06]

pkotwicz@chromium.org changed reviewers:
+ tfarina@chromium.org
- jamescook@chormium.org

[pkotwicz, 2014-09-24 02:07:35]

tfarina@ can you please take a look? I think that jamescook@ is out of office

[tfarina, 2014-09-24 13:14:50]

Hi Peter, I will take a look at this later today.

But I agree that you should get approval from Oshima as well.

Regards,

[pkotwicz, 2014-09-24 13:40:53]

pkotwicz@chromium.org changed reviewers:
+ oshima@chromium.org

[pkotwicz, 2014-09-24 13:40:54]

Oshima, can you please take a look at the changes in athena?

[oshima, 2014-09-24 17:55:19]

athena/ lgtm

alternative way is to put grd/gyp in athena/strings, and put translations in
athena/strings/translations. (this seems to be what ui/strings does) I'm fine
with either way.

https://codereview.chromium.org/591803002/diff/100001/tools/gritsettings/reso...
File tools/gritsettings/resource_ids (right):

https://codereview.chromium.org/591803002/diff/100001/tools/gritsettings/reso...
tools/gritsettings/resource_ids:227: "messages": [42000],
isn't it better to put this before blink resources (probably next to ash)?

[tfarina, 2014-09-24 17:58:23]

On Wed, Sep 24, 2014 at 2:55 PM, <oshima@chromium.org> wrote:

> athena/ lgtm
>
> alternative way is to put grd/gyp in athena/strings, and put translations
> in
> athena/strings/translations. (this seems to be what ui/strings does) I'm
> fine
> with either way.
>
Yeah, please, could you do this way? *.xtb files go into
athena/strings/translations/

-- 
Thiago Farina

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[tfarina, 2014-09-24 18:00:49]

https://codereview.chromium.org/591803002/diff/100001/athena/athena_strings.gyp
File athena/athena_strings.gyp (right):

https://codereview.chromium.org/591803002/diff/100001/athena/athena_strings.g...
athena/athena_strings.gyp:22: 'includes': [ '../build/grit_target.gypi' ],
could you remove this? We are trying to use qualified paths for grit includes.
See crbug.com/401588

[pkotwicz, 2014-09-24 20:26:09]

Patchset #4 (id:120001) has been deleted

[pkotwicz, 2014-09-24 20:39:36]

Patchset #4 (id:140001) has been deleted

[pkotwicz, 2014-09-24 20:39:55]

tfarina@ can you please take another look?

[pkotwicz, 2014-09-24 20:40:32]

Patchset #4 (id:160001) has been deleted

[pkotwicz, 2014-09-24 20:44:48]

https://codereview.chromium.org/591803002/diff/100001/tools/gritsettings/reso...
File tools/gritsettings/resource_ids (right):

https://codereview.chromium.org/591803002/diff/100001/tools/gritsettings/reso...
tools/gritsettings/resource_ids:227: "messages": [42000],
I do not want to get too close to the max id used by ash_resources.grd
There is not a lot of room between ash_resouces.grd and athena_resourecs.grd I
did not feel like changing the IDs for all of the .grd files

[tfarina, 2014-09-24 22:38:49]

athena/strings lgtm.

Could you also look into adding GN build file there as well? 

You can take a look at ui/strings/BUILD.gn for an example of how you could do
it.

https://chromium.googlesource.com/chromium/src/+/master/ui/strings/BUILD.gn

Thanks,

https://codereview.chromium.org/591803002/diff/200001/athena/strings/athena_s...
File athena/strings/athena_strings.grd (right):

https://codereview.chromium.org/591803002/diff/200001/athena/strings/athena_s...
athena/strings/athena_strings.grd:106: <!-- TODO(pkotwicz): Add Placeholder
translations. See crbug.com/416209 -->
right. thanks adding this TODO.

[oshima, 2014-09-25 00:41:59]

https://codereview.chromium.org/591803002/diff/100001/tools/gritsettings/reso...
File tools/gritsettings/resource_ids (right):

https://codereview.chromium.org/591803002/diff/100001/tools/gritsettings/reso...
tools/gritsettings/resource_ids:227: "messages": [42000],
On 2014/09/24 20:44:48, pkotwicz wrote:
> I do not want to get too close to the max id used by ash_resources.grd
> There is not a lot of room between ash_resouces.grd and athena_resourecs.grd I
> did not feel like changing the IDs for all of the .grd files

You can just update the ids after ash

[oshima, 2014-09-25 00:43:00]

slgtm

On 2014/09/25 00:41:59, oshima wrote:
>
https://codereview.chromium.org/591803002/diff/100001/tools/gritsettings/reso...
> File tools/gritsettings/resource_ids (right):
> 
>
https://codereview.chromium.org/591803002/diff/100001/tools/gritsettings/reso...
> tools/gritsettings/resource_ids:227: "messages": [42000],
> On 2014/09/24 20:44:48, pkotwicz wrote:
> > I do not want to get too close to the max id used by ash_resources.grd
> > There is not a lot of room between ash_resouces.grd and athena_resourecs.grd
I
> > did not feel like changing the IDs for all of the .grd files
> 
wanted to say:

You can just update the ids after ash *safely*.

[tony, 2014-09-25 00:49:37]

rubber stamp LGTM

[tony, 2014-09-25 00:51:42]

https://codereview.chromium.org/591803002/diff/200001/tools/gritsettings/reso...
File tools/gritsettings/resource_ids (right):

https://codereview.chromium.org/591803002/diff/200001/tools/gritsettings/reso...
tools/gritsettings/resource_ids:15: # common convention starts practical use of
IDs at 100 or 101.
We might want to update this comment since we no longer use Windows .rc files
and pak files can go up to 64k.

[pkotwicz, 2014-09-25 01:33:44]

https://codereview.chromium.org/591803002/diff/200001/tools/gritsettings/reso...
File tools/gritsettings/resource_ids (right):

https://codereview.chromium.org/591803002/diff/200001/tools/gritsettings/reso...
tools/gritsettings/resource_ids:15: # common convention starts practical use of
IDs at 100 or 101.
What do you suggest changing the comment to?

[tony, 2014-09-25 03:35:21]

https://codereview.chromium.org/591803002/diff/200001/tools/gritsettings/reso...
File tools/gritsettings/resource_ids (right):

https://codereview.chromium.org/591803002/diff/200001/tools/gritsettings/reso...
tools/gritsettings/resource_ids:15: # common convention starts practical use of
IDs at 100 or 101.
On 2014/09/25 01:33:44, pkotwicz wrote:
> What do you suggest changing the comment to?

"The range of ID values, which is used by pak files, is from 0 to 2^16 - 1."

[pkotwicz, 2014-09-25 15:10:00]

pkotwicz@chromium.org changed reviewers:
+ sky@chromium.org

[pkotwicz, 2014-09-25 15:10:01]

sky for chrome/ OWNERS

[sky, 2014-09-25 19:33:05]

LGTM

[pkotwicz, 2014-09-25 20:19:36]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-09-25 20:21:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/591803002/220001

[commit-bot: I haz the power, 2014-09-25 21:34:16]

Committed patchset #6 (id:220001) as 66c8c962c0f4fdca2737bc32b0abd9a7c2793344

[commit-bot: I haz the power, 2014-09-25 21:35:45]

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