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

CLDR-17141 kbd: keep transform=no but document it #3332

Conversation

srl295
Copy link
Member

@srl295 srl295 commented Oct 10, 2023

CLDR-17141

ALLOW_MANY_COMMITS=true

@srl295 srl295 requested review from miloush and a team October 10, 2023 20:01
@srl295 srl295 self-assigned this Oct 10, 2023
@srl295 srl295 mentioned this pull request Oct 11, 2023
1 task
@mcdurdin
Copy link
Contributor

I'd prefer to drop transform=no and simplify the mental model for keyboard authors. transform=no may seem simpler, but the caveats mean they have to have some grasp of transforms anyway, and then it just becomes an additional layer to remember.

@srl295
Copy link
Member Author

srl295 commented Oct 11, 2023

@jahorton fyi

@srl295 srl295 force-pushed the srl295/kbd/cldr-17141-drop-transform-no branch from 6ee3b50 to 00b938b Compare October 11, 2023 21:04
@srl295 srl295 force-pushed the srl295/kbd/cldr-17141-dont-drop-transform-no branch from ee8dc9c to 32fe1fd Compare October 11, 2023 21:05
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • common/supplemental/attributeValueValidity.xml is no longer changed in the branch
  • docs/ldml/tr35-keyboards.md is different
  • keyboards/3.0/fr-t-k0-azerty.xml is no longer changed in the branch
  • keyboards/3.0/ja-Latn.xml is no longer changed in the branch
  • keyboards/3.0/mt-t-k0-47key.xml is no longer changed in the branch
  • keyboards/3.0/mt.xml is no longer changed in the branch
  • keyboards/3.0/pcm.xml is no longer changed in the branch
  • keyboards/3.0/pt-t-k0-abnt2.xml is no longer changed in the branch
  • keyboards/dtd/ldmlKeyboard3.xsd is no longer changed in the branch
  • keyboards/import/keys-Latn-implied.xml is no longer changed in the branch
  • keyboards/import/keys-Zyyy-currency.xml is no longer changed in the branch
  • keyboards/import/keys-Zyyy-punctuation.xml is no longer changed in the branch
  • tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestDtdData.java is no longer changed in the branch
  • tools/cldr-code/src/test/resources/org/unicode/cldr/tool/KeyboardFlatten/broken-import-missing.xml is no longer changed in the branch
  • tools/cldr-code/src/test/resources/org/unicode/cldr/tool/KeyboardFlatten/broken-import-unknownbase.xml is no longer changed in the branch
  • tools/cldr-code/src/test/resources/org/unicode/cldr/tool/KeyboardFlatten/broken-import-unknownver.xml is no longer changed in the branch
  • tools/cldr-code/src/test/resources/org/unicode/cldr/tool/KeyboardFlatten/broken-import-wrongparent.xml is no longer changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@srl295
Copy link
Member Author

srl295 commented Oct 11, 2023

@miloush thoughts here? @mcdurdin requesting to drop (meaning i can just remove this PR).

I agree that the complexity and limitations of transform=no make it very limited use, especially this:

the caveats mean they have to have some grasp of transforms anyway

As "syntactic sugar" it may not add a lot. Probably better to work on documentation, and tooling (including debugging and test environments, test cases, etc) for transforms.

@srl295
Copy link
Member Author

srl295 commented Oct 11, 2023

@miloush Are you OK if we close this one (that is, drop transform=no) and merge #3330 ?

@miloush
Copy link
Contributor

miloush commented Oct 11, 2023

Oh sorry I meant yes, feel free to drop.

@srl295 srl295 merged commit 0ebdc9e into srl295/kbd/cldr-17141-drop-transform-no Oct 11, 2023
8 checks passed
@srl295 srl295 deleted the srl295/kbd/cldr-17141-dont-drop-transform-no branch October 11, 2023 21:31
@srl295
Copy link
Member Author

srl295 commented Oct 11, 2023

@miloush sorry, i merged the wrong PR 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants