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.

Sign up for GitHub

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

Jump to bottom

CLDR-16438 document filtering out unneeded data #2761

Closed
wants to merge 15 commits into from
Closed

CLDR-16438 document filtering out unneeded data #2761

wants to merge 15 commits into from

Conversation

macchiati
Copy link
Member

This is for tickets https://unicode-org.atlassian.net/browse/CLDR-16438 and https://unicode-org.atlassian.net/browse/CLDR-16340.

CLDR-16438

  • This PR completes the ticket.

ALLOW_MANY_COMMITS=true

btangmu and others added 8 commits February 28, 2023 12:33
@btangmu
CLDR-16192 Admin email encoding failure for non-ASCII text (#2599)
927fa15 
-Simply return parameter unchanged for WebContext.decodeFieldString

-Comments
@btangmu
CLDR-11368 Enable Forum posting by TC even when read-only (#2754)
c3f2c45 
-Revise SurveyForum.userCanUsePostType, isPhaseReadonly only matters for non-TC

-Refactor with boolean isTC to avoid calling userIsTC repeatedly
@srl295
CLDR-16437 v44 BRS: dependabot bump for v44 (#2753)
c4e3ee8
@dependabot
CLDR-16437 gh:(deps): Bump actions/cache from 2 to 3 (#2757)
994a322 
Bumps [actions/cache](https://github.com/actions/cache) from 2 to 3.
- [Release notes](https://github.com/actions/cache/releases)
- [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md)
- [Commits](actions/cache@v2...v3)

---
updated-dependencies:
- dependency-name: actions/cache
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot
CLDR-16437 j:(deps): Bump json from 20190722 to 20230227 in /tools (#…
1de5706 
…2756)

Bumps [json](https://github.com/douglascrockford/JSON-java) from 20190722 to 20230227.
- [Release notes](https://github.com/douglascrockford/JSON-java/releases)
- [Changelog](https://github.com/stleary/JSON-java/blob/master/docs/RELEASES.md)
- [Commits](https://github.com/douglascrockford/JSON-java/commits)

---
updated-dependencies:
- dependency-name: org.json:json
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@macchiati
CLDR-16438 document filtering out unneeded data
5970d07 
This is for tickets https://unicode-org.atlassian.net/browse/CLDR-16438 and https://unicode-org.atlassian.net/browse/CLDR-16340.
@macchiati
CLDR-16438 Add code snippet
1a26448
@macchiati
CLDR-16438 Fix formatting
c322dab
@macchiati macchiati marked this pull request as ready for review March 6, 2023 02:22
srl295
srl295 previously approved these changes Mar 6, 2023
View reviewed changes
Copy link
Member

@srl295 srl295 left a comment

Choose a reason for hiding this comment

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

Definite improvement.

At some point we should make sure the wording here is harmonized with https://cldr.unicode.org/index/cldr-spec/coverage-levels for example.

Also the Coverage level section I linked to says:

That structure is primarily intended for internal use in CLDR tooling — it is not anticipated that users of CLDR data would need it.

May need to revise the above if they are no longer internal.

docs/ldml/tr35.md Outdated Show resolved Hide resolved
docs/ldml/tr35.md Outdated Show resolved Hide resolved
@macchiati @srl295
CLDR-16438 typos
e4f580d 
Co-authored-by: Steven R. Loomis <srl295@gmail.com>
macchiati and others added 2 commits March 6, 2023 09:23
@macchiati @srl295
CLDR-16438 typos
433eff5 
Co-authored-by: Steven R. Loomis <srl295@gmail.com>
@macchiati
CLDR-16438 Update tr35-info also
266427d
@macchiati macchiati requested a review from srl295 March 6, 2023 17:56
srl295
srl295 previously approved these changes Mar 6, 2023
View reviewed changes
Copy link
Member

@srl295 srl295 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM. Excellent addition to the spec, very consumer oriented. I expect as with any newer 'public surface' of the project we may have some bumps and questions but they can shake out over time.

sffc
sffc requested changes Mar 6, 2023
View reviewed changes
docs/ldml/tr35.md Outdated Show resolved Hide resolved
docs/ldml/tr35.md Outdated Show resolved Hide resolved
docs/ldml/tr35.md Outdated Show resolved Hide resolved
@macchiati @sffc
CLDR-16438 typos
a5929c0 
Co-authored-by: Shane F. Carr <shane@unicode.org>
@macchiati macchiati requested review from sffc and srl295 March 7, 2023 01:08
sffc
sffc previously approved these changes Mar 7, 2023
View reviewed changes
Copy link
Member

@sffc sffc left a comment

Choose a reason for hiding this comment

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

This looks good. Definitely good to explicitly state this in the spec.

I think there's room to improve this section further (more emphasis on explicit algorithms rather than left-to-the-reader guidelines), but that needs more thought and doesn't need to be done right away.

@macchiati macchiati changed the base branch from main to maint/maint-43 March 7, 2023 01:53
@macchiati macchiati dismissed sffc’s stale review March 7, 2023 01:53

The base branch was changed.

@macchiati macchiati requested a review from sffc March 7, 2023 01:53
@macchiati
Copy link
Member Author

Sorry, had wrong target. Can you restamp?

@macchiati
Copy link
Member Author

Messed up in github, so created new PR #2766

@macchiati macchiati closed this Mar 7, 2023
@macchiati macchiati mentioned this pull request Mar 7, 2023
Merged
1 task
@macchiati
Copy link
Member Author

macchiati commented Mar 7, 2023 via email

@srl295
Copy link
Member

srl295 commented Mar 7, 2023

Good. I agree that we can flesh this out over time. We might even want to break out a new "Part" that has implementation techniques, where we can gather them all together.

It's a trade off. That was the goal of the implementers guide ( stub | toc ) … I'm not sure the TR is the right place for too much implementation guidance vs. some more informal cookbook. However, no other vehicles have 'stuck' in the meantime.

@macchiati
Copy link
Member Author

We could have an additional github document, eg LDML Implementers Guide

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pedberg-icu pedberg-icu Awaiting requested review from pedberg-icu

@srl295 srl295 Awaiting requested review from srl295

@sffc sffc Awaiting requested review from sffc

Assignees

@macchiati macchiati

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@macchiati @srl295 @sffc @btangmu