[Chromecast] Remove chromecast_repack_locales.py.

[Chromecast] Remove chromecast_repack_locales.py.

This script predates GN templates, which allow us to do all of our
resource packing in GN. Eliminate the script and correct some bad out
directory conventions.

BUG=697090
TEST= diff the .pak output before and after the change <= no diff

Review-Url: https://codereview.chromium.org/2773453003
Cr-Commit-Position: refs/heads/master@{#459200}
Committed: https://chromium.googlesource.com/chromium/src/+/5c5624b304dd7710b1422a16b7d4039c0dcef283

[slan, 2017-03-22 20:15:26]

slan@chromium.org changed reviewers:
+ alokp@chromium.org, mbjorge@chromium.org, thakis@chromium.org

[slan, 2017-03-22 20:15:26]

With GN, we don't need this hacky script. Woohoo!

[Nico, 2017-03-22 20:17:10]

lgtm!

[slan, 2017-03-22 20:52:21]

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

[commit-bot: I haz the power, 2017-03-22 20:53:00]

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

[mbjorge, 2017-03-22 21:26:33]

Woo! Hoorary for deleting things

https://codereview.chromium.org/2773453003/diff/1/chromecast/BUILD.gn
File chromecast/BUILD.gn (right):

https://codereview.chromium.org/2773453003/diff/1/chromecast/BUILD.gn#newcode428
chromecast/BUILD.gn:428: [
"$root_gen_dir/chromecast_strings/app_strings_${locale}.pak" ]
Any benefit in moving the webui strings to output to the default directory also,
instead of /chromecast_strings?

https://codereview.chromium.org/2773453003/diff/1/chromecast/BUILD.gn#newcode447
chromecast/BUILD.gn:447: cast_repack_locales("chromecast_locales_pak") {
Is the template actually necessary?

Could you just have:

foreach(locale, locales) {
  repack("_cast_repack_${locale}") {
     output = "$root_out_dir/chromecast_locales/${locale}.pak"
     sources = ...
     ...
  }
}

group("chromecast_locales_pak") {
  foreach(locale, locales) {
     deps += [ ":_cast_repack_${locale}" ]
  }
}

https://codereview.chromium.org/2773453003/diff/1/chromecast/app/BUILD.gn
File chromecast/app/BUILD.gn (right):

https://codereview.chromium.org/2773453003/diff/1/chromecast/app/BUILD.gn#new...
chromecast/app/BUILD.gn:108: outputs = [
should this be based on the |locales| array of suffixes declared in
//build/config/locales.gni also?

[commit-bot: I haz the power, 2017-03-22 22:06:34]

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

[commit-bot: I haz the power, 2017-03-22 22:06:35]

Dry run: This issue passed the CQ dry run.

[slan, 2017-03-22 22:54:52]

Great comments, Mike. PTAL.

https://codereview.chromium.org/2773453003/diff/1/chromecast/BUILD.gn
File chromecast/BUILD.gn (right):

https://codereview.chromium.org/2773453003/diff/1/chromecast/BUILD.gn#newcode428
chromecast/BUILD.gn:428: [
"$root_gen_dir/chromecast_strings/app_strings_${locale}.pak" ]
On 2017/03/22 21:26:33, mbjorge wrote:
> Any benefit in moving the webui strings to output to the default directory
also,
> instead of /chromecast_strings?

Yeah, ok... Will need a corresponding internal change. Done.

https://codereview.chromium.org/2773453003/diff/1/chromecast/BUILD.gn#newcode447
chromecast/BUILD.gn:447: cast_repack_locales("chromecast_locales_pak") {
On 2017/03/22 21:26:33, mbjorge wrote:
> Is the template actually necessary?
> 
> Could you just have:
> 
> foreach(locale, locales) {
>   repack("_cast_repack_${locale}") {
>      output = "$root_out_dir/chromecast_locales/${locale}.pak"
>      sources = ...
>      ...
>   }
> }
> 
> group("chromecast_locales_pak") {
>   foreach(locale, locales) {
>      deps += [ ":_cast_repack_${locale}" ]
>   }
> }

Yes, I can. I think it's cleaner too. Brilliant!

https://codereview.chromium.org/2773453003/diff/1/chromecast/app/BUILD.gn
File chromecast/app/BUILD.gn (right):

https://codereview.chromium.org/2773453003/diff/1/chromecast/app/BUILD.gn#new...
chromecast/app/BUILD.gn:108: outputs = [
On 2017/03/22 21:26:33, mbjorge wrote:
> should this be based on the |locales| array of suffixes declared in
> //build/config/locales.gni also?

Great idea! Done.

[slan, 2017-03-22 22:56:23]

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

[commit-bot: I haz the power, 2017-03-22 22:56:56]

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, 2017-03-23 01:35:42]

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

[commit-bot: I haz the power, 2017-03-23 01:35:43]

Dry run: This issue passed the CQ dry run.

[slan, 2017-03-23 19:59:04]

Description was changed from

==========
[Chromecast] Remove chromecast_repack_locales.py.

This script predates GN templates, which allow us to do all of our
resource packing in GN. Eliminate the script and correct some bad out
directory conventions.

BUG=697090
TEST= diff the .pak output before and after the change <= no diff

Change-Id: I3a8b416339edfdd61b97a18f4cd80046f57cbb0a
==========

to

==========
[Chromecast] Remove chromecast_repack_locales.py.

This script predates GN templates, which allow us to do all of our
resource packing in GN. Eliminate the script and correct some bad out
directory conventions.

BUG=697090
TEST= diff the .pak output before and after the change <= no diff
==========

[alokp, 2017-03-23 20:11:12]

lgtm

[slan, 2017-03-23 20:14:59]

The CQ bit was checked by slan@chromium.org

[slan, 2017-03-23 20:14:59]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/2773453003/#ps20001
(title: "Even more clean-up!")

[slan, 2017-03-23 20:15:35]

slan@chromium.org changed reviewers:
+ gfhuang@chromium.org

[slan, 2017-03-23 20:15:35]

+gfhuang FYI

The internal review will be coming your way, Cast folks.

[commit-bot: I haz the power, 2017-03-23 20:15:43]

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, 2017-03-23 20:24:19]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490300099255250,
"parent_rev": "0bd994d15c644515a66f1ddd081122894e314e3b", "commit_rev":
"5c5624b304dd7710b1422a16b7d4039c0dcef283"}

[commit-bot: I haz the power, 2017-03-23 20:25:01]

Description was changed from

==========
[Chromecast] Remove chromecast_repack_locales.py.

This script predates GN templates, which allow us to do all of our
resource packing in GN. Eliminate the script and correct some bad out
directory conventions.

BUG=697090
TEST= diff the .pak output before and after the change <= no diff
==========

to

==========
[Chromecast] Remove chromecast_repack_locales.py.

This script predates GN templates, which allow us to do all of our
resource packing in GN. Eliminate the script and correct some bad out
directory conventions.

BUG=697090
TEST= diff the .pak output before and after the change <= no diff

Review-Url: https://codereview.chromium.org/2773453003
Cr-Commit-Position: refs/heads/master@{#459200}
Committed:
https://chromium.googlesource.com/chromium/src/+/5c5624b304dd7710b1422a16b7d4...
==========

[commit-bot: I haz the power, 2017-03-23 20:25:03]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/5c5624b304dd7710b1422a16b7d4...