1150733 - Correctly internationalize sample label in jit coach

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), defect)

Description

[Jordan Santell [:jsantell] [@jsantell]]

From bug 1143933#comment35

Comment 1

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1150733-i10n-jit.patch

Comment 2

[Victor Porof [:vporof][:vp]]

Comment on attachment 8587714 [details] [diff] [review]
1150733-i10n-jit.patch

Review of attachment 8587714 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/performance/views/jit-optimizations.js
@@ +209,5 @@
>        node.appendChild(icon);
>      }
>  
> +    let sampleString = PluralForm.get(site.samples, JIT_SAMPLES).replace("#1", site.samples);
> +    desc.textContent = `${lastStrategy} - (${sampleString})`;

You want a dash here (–), not a minus (-).

Comment 3

[Francesco Lodolo [:flod]]

Comment on attachment 8587714 [details] [diff] [review]
1150733-i10n-jit.patch

Review of attachment 8587714 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/locales/en-US/chrome/browser/devtools/profiler.properties
@@ +130,1 @@
>  # This string is displayed for the unit representing thenumber of times a

Still typo here: "the number"

@@ +130,5 @@
>  # This string is displayed for the unit representing thenumber of times a
>  # frame is sampled.
> +# #1 sample
> +# example: 30 samples
> +jit.samples=#1 sample;#1 samples

Sorry but you need to change the string ID at this point
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings

Comment 4

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1150733-i10n-jit.patch

Made the remaining changes
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2e9145c3f839

Comment 5

[Francesco Lodolo [:flod]]

Comment on attachment 8588404 [details] [diff] [review]
1150733-i10n-jit.patch

Review of attachment 8588404 [details] [diff] [review]:
-----------------------------------------------------------------

Switching to feedback, I'm not really entitled to review patches with code (I guess you can carry on r+ from the previous patch though).

::: browser/locales/en-US/chrome/browser/devtools/profiler.properties
@@ +131,2 @@
>  # frame is sampled.
> +# #1 sample

I think it's safe to drop this line, or "#1 represents the number of sample" (I completely missed it in the previous check).

Comment 6

[Francesco Lodolo [:flod]]

(In reply to Francesco Lodolo [:flod] (UTC+2) from comment #5)
> "#1 represents the number of sample"
s/sample/samples

Comment 7

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1150733-i10n-jit.patch

Thanks for the feedback, Francesco!

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/12f07c1a9e0e

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/12f07c1a9e0e