Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[localize] Fix and document msgdesc comments #1445

Merged
merged 3 commits into from Dec 4, 2020
Merged

Conversation

aomarks
Copy link
Member

@aomarks aomarks commented Nov 24, 2020

  • Fix an XML serialization bug that broke // msgdesc: comments.
  • Add some test cases.
  • Document them in the README.
  • Fix incorrect README transform vs runtime table.

Fixes #1444

@github-actions
Copy link
Contributor

github-actions bot commented Nov 24, 2020

📊 Tachometer Benchmark Results

Summary

nop-update

  • lit-html-kitchen-sink: unsure 🔍 -4% - +6% (-1.29ms - +2.12ms)
    this-change vs tip-of-tree

render

  • lit-element-list: unsure 🔍 -0% - +1% (-0.26ms - +0.99ms)
    this-change vs tip-of-tree
  • lit-html-kitchen-sink: unsure 🔍 -1% - +0% (-0.39ms - +0.15ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -1% - +1% (-0.07ms - +0.08ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -0% - +2% (-0.32ms - +1.15ms)
    this-change vs tip-of-tree
  • updating-element-list: unsure 🔍 -1% - +0% (-0.94ms - +0.22ms)
    this-change vs tip-of-tree

update

  • lit-element-list: slower ❌ 0% - 1% (0.14ms - 12.82ms)
    this-change vs tip-of-tree
  • lit-html-kitchen-sink: unsure 🔍 -3% - +2% (-3.00ms - +2.36ms)
    this-change vs tip-of-tree
  • lit-html-repeat: unsure 🔍 -1% - +1% (-2.28ms - +1.97ms)
    this-change vs tip-of-tree
  • lit-html-template-heavy: unsure 🔍 -1% - +2% (-1.35ms - +2.57ms)
    this-change vs tip-of-tree
  • updating-element-list: unsure 🔍 -1% - +1% (-6.73ms - +11.58ms)
    this-change vs tip-of-tree

update-reflect

  • lit-element-list: unsure 🔍 -0% - +1% (-4.49ms - +9.21ms)
    this-change vs tip-of-tree
  • updating-element-list: unsure 🔍 -1% - +1% (-12.30ms - +8.02ms)
    this-change vs tip-of-tree

Results

lit-element-list

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
93.05ms - 94.09ms-unsure 🔍
-0% - +1%
-0.26ms - +0.99ms
faster ✔
22% - 24%
27.25ms - 28.83ms
tip-of-tree
tip-of-tree
92.87ms - 93.54msunsure 🔍
-1% - +0%
-0.99ms - +0.26ms
-faster ✔
23% - 24%
27.72ms - 29.08ms
previous-release
previous-release
121.01ms - 122.20msslower ❌
29% - 31%
27.25ms - 28.83ms
slower ❌
30% - 31%
27.72ms - 29.08ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
895.38ms - 904.48ms-slower ❌
0% - 1%
0.14ms - 12.82ms
faster ✔
7% - 8%
68.52ms - 82.37ms
tip-of-tree
tip-of-tree
889.04ms - 897.87msfaster ✔
0% - 1%
0.14ms - 12.82ms
-faster ✔
8% - 9%
75.08ms - 88.77ms
previous-release
previous-release
970.15ms - 980.60msslower ❌
8% - 9%
68.52ms - 82.37ms
slower ❌
8% - 10%
75.08ms - 88.77ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
968.32ms - 978.19ms-unsure 🔍
-0% - +1%
-4.49ms - +9.21ms
faster ✔
4% - 5%
38.03ms - 53.17ms
tip-of-tree
tip-of-tree
966.14ms - 975.64msunsure 🔍
-1% - +0%
-9.21ms - +4.49ms
-faster ✔
4% - 5%
40.51ms - 55.41ms
previous-release
previous-release
1013.11ms - 1024.60msslower ❌
4% - 5%
38.03ms - 53.17ms
slower ❌
4% - 6%
40.51ms - 55.41ms
-
lit-html-kitchen-sink

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
37.98ms - 38.24ms-unsure 🔍
-1% - +0%
-0.39ms - +0.15ms
faster ✔
20% - 22%
9.52ms - 10.55ms
tip-of-tree
tip-of-tree
37.99ms - 38.47msunsure 🔍
-0% - +1%
-0.15ms - +0.39ms
-faster ✔
20% - 22%
9.36ms - 10.46ms
previous-release
previous-release
47.64ms - 48.63msslower ❌
25% - 28%
9.52ms - 10.55ms
slower ❌
24% - 27%
9.36ms - 10.46ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
94.48ms - 98.27ms-unsure 🔍
-3% - +2%
-3.00ms - +2.36ms
unsure 🔍
-2% - +3%
-1.72ms - +2.87ms
tip-of-tree
tip-of-tree
94.80ms - 98.58msunsure 🔍
-2% - +3%
-2.36ms - +3.00ms
-unsure 🔍
-1% - +3%
-1.40ms - +3.19ms
previous-release
previous-release
94.50ms - 97.09msunsure 🔍
-3% - +2%
-2.87ms - +1.72ms
unsure 🔍
-3% - +1%
-3.19ms - +1.40ms
-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
32.72ms - 35.90ms-unsure 🔍
-4% - +6%
-1.29ms - +2.12ms
unsure 🔍
-12% - +0%
-4.35ms - +0.08ms
tip-of-tree
tip-of-tree
33.27ms - 34.52msunsure 🔍
-6% - +4%
-2.12ms - +1.29ms
-faster ✔
3% - 11%
0.89ms - 4.21ms
previous-release
previous-release
34.91ms - 37.98msunsure 🔍
-0% - +13%
-0.08ms - +4.35ms
slower ❌
3% - 12%
0.89ms - 4.21ms
-
lit-html-repeat

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
12.01ms - 12.12ms-unsure 🔍
-1% - +1%
-0.07ms - +0.08ms
faster ✔
18% - 19%
2.74ms - 2.90ms
tip-of-tree
tip-of-tree
12.01ms - 12.10msunsure 🔍
-1% - +1%
-0.08ms - +0.07ms
-faster ✔
19% - 19%
2.76ms - 2.90ms
previous-release
previous-release
14.83ms - 14.94msslower ❌
23% - 24%
2.74ms - 2.90ms
slower ❌
23% - 24%
2.76ms - 2.90ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
363.87ms - 366.94ms-unsure 🔍
-1% - +1%
-2.28ms - +1.97ms
faster ✔
31% - 33%
167.90ms - 175.95ms
tip-of-tree
tip-of-tree
364.09ms - 367.03msunsure 🔍
-1% - +1%
-1.97ms - +2.28ms
-faster ✔
31% - 33%
167.77ms - 175.78ms
previous-release
previous-release
533.61ms - 541.06msslower ❌
46% - 48%
167.90ms - 175.95ms
slower ❌
46% - 48%
167.77ms - 175.78ms
-
lit-html-template-heavy

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
67.15ms - 68.04ms-unsure 🔍
-0% - +2%
-0.32ms - +1.15ms
faster ✔
14% - 16%
11.02ms - 12.55ms
tip-of-tree
tip-of-tree
66.59ms - 67.77msunsure 🔍
-2% - +0%
-1.15ms - +0.32ms
-faster ✔
14% - 16%
11.34ms - 13.06ms
previous-release
previous-release
78.76ms - 80.01msslower ❌
16% - 19%
11.02ms - 12.55ms
slower ❌
17% - 20%
11.34ms - 13.06ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
139.89ms - 142.63ms-unsure 🔍
-1% - +2%
-1.35ms - +2.57ms
faster ✔
13% - 16%
21.38ms - 25.94ms
tip-of-tree
tip-of-tree
139.24ms - 142.04msunsure 🔍
-2% - +1%
-2.57ms - +1.35ms
-faster ✔
13% - 16%
21.97ms - 26.57ms
previous-release
previous-release
163.09ms - 166.74msslower ❌
15% - 18%
21.38ms - 25.94ms
slower ❌
16% - 19%
21.97ms - 26.57ms
-
updating-element-list

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
66.89ms - 67.63ms-unsure 🔍
-1% - +0%
-0.94ms - +0.22ms
unsure 🔍
-1% - +1%
-0.53ms - +0.55ms
tip-of-tree
tip-of-tree
67.17ms - 68.07msunsure 🔍
-0% - +1%
-0.22ms - +0.94ms
-unsure 🔍
-0% - +1%
-0.22ms - +0.96ms
previous-release
previous-release
66.86ms - 67.64msunsure 🔍
-1% - +1%
-0.55ms - +0.53ms
unsure 🔍
-1% - +0%
-0.96ms - +0.22ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
935.97ms - 949.89ms-unsure 🔍
-1% - +1%
-6.73ms - +11.58ms
unsure 🔍
-1% - +1%
-7.37ms - +10.53ms
tip-of-tree
tip-of-tree
934.56ms - 946.45msunsure 🔍
-1% - +1%
-11.58ms - +6.73ms
-unsure 🔍
-1% - +1%
-9.03ms - +7.34ms
previous-release
previous-release
935.72ms - 946.98msunsure 🔍
-1% - +1%
-10.53ms - +7.37ms
unsure 🔍
-1% - +1%
-7.34ms - +9.03ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
1029.20ms - 1044.88ms-unsure 🔍
-1% - +1%
-12.30ms - +8.02ms
unsure 🔍
-1% - +1%
-13.95ms - +6.74ms
tip-of-tree
tip-of-tree
1032.72ms - 1045.65msunsure 🔍
-1% - +1%
-8.02ms - +12.30ms
-unsure 🔍
-1% - +1%
-10.81ms - +7.89ms
previous-release
previous-release
1033.89ms - 1047.40msunsure 🔍
-1% - +1%
-6.74ms - +13.95ms
unsure 🔍
-1% - +1%
-7.89ms - +10.81ms
-

tachometer-reporter-action v2 for Benchmarks

Base automatically changed from localize-optional-2 to lit-next December 4, 2020 18:52
@@ -250,9 +250,10 @@ export class XliffFormatter implements Formatter {

if (descStack.length > 0) {
// https://docs.oasis-open.org/xliff/v1.2/os/xliff-core.html#note
const note = document.createElement('note');
const note = doc.createElement('note');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ouch!

Can you remove the "DOM" lib from the tsconfig? Hopefully TS would catch this.

Copy link
Member Author

@aomarks aomarks Dec 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. (But DOM types still get loaded because the xmldom package I'm loading has a reference to them)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aomarks aomarks merged commit d341b51 into lit-next Dec 4, 2020
@aomarks aomarks deleted the localize-msgdesc branch December 4, 2020 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants