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

1.0 PRD and Roadmap documents #665

Merged
merged 4 commits into from
Apr 26, 2021
Merged

Conversation

zbraniecki
Copy link
Member

Extracted the doc we aligned on into markdown.

I decided to separate PRD from Roadmap to make the roadmap easier to check quickly, while the PRD is more of a whitepaper behind it and will likely be read and updated less often and maybe even by a different audience.

@zbraniecki zbraniecki requested a review from a team as a code owner April 20, 2021 20:38
@zbraniecki zbraniecki added this to the ICU4X 0.2 milestone Apr 20, 2021
@zbraniecki zbraniecki requested review from nciric and removed request for a team April 20, 2021 20:39
@zbraniecki zbraniecki linked an issue Apr 20, 2021 that may be closed by this pull request
@coveralls
Copy link

coveralls commented Apr 20, 2021

Pull Request Test Coverage Report for Build 00beb40611d4fa8e3592af19d59bc6ecfb7a41fb-PR-665

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 72 unchanged lines in 6 files lost coverage.
  • Overall coverage increased (+0.1%) to 73.492%

Files with Coverage Reduction New Missed Lines %
utils/zerovec/src/varzerovec/mod.rs 3 60.61%
utils/zerovec/src/ule/chars.rs 4 77.55%
utils/zerovec/src/zerovec/mod.rs 8 67.0%
utils/zerovec/src/ule/plain.rs 11 0%
utils/zerovec/src/varzerovec/components.rs 13 81.48%
components/provider_cldr/src/transform/dates.rs 33 73.18%
Totals Coverage Status
Change from base Build a502f58f0c3529d6aa4bf7fed1f43973f2b4818a: 0.1%
Covered Lines: 8162
Relevant Lines: 11106

💛 - Coveralls

docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
* [x] Unicode Set
* [x] L3a
* [ ] March/April
* [ ] FFI/WASM exploration
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this got moved to May (0.3)

Copy link
Member Author

Choose a reason for hiding this comment

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

I assume it spans both since Manish and Shane are working on it in April, so I added it to both :)

Copy link
Member Author

Choose a reason for hiding this comment

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

lmk if that works!

docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
- [Intl.PluralRules](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/PluralRules)
- [Intl.DateTimeFormat](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/DateTimeFormat)

The former two should be able to pass the full test scope.
Copy link
Contributor

Choose a reason for hiding this comment

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

The 262 test suite has lots of JS specific elements, like is returned value an Object/Array etc. I assume we are not interested in fulfilling those.

Copy link
Member Author

Choose a reason for hiding this comment

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

We are not, but I assume we want sticking ICU4X into a JS implementation (SpiderMonkey, Deno, V8 etc.) ergonomic so that they only have to care about Object/Array etc.

Do you think it should be stated explicitly here? If so, do you have a suggestion how to phrase it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should commit it as is, and I'll edit (with your review) afterwards?


### Fuchsia

Fuchsia maintains [a thin wrapper around ICU4C](https://crates.io/crates/rust_icu) exposed to Rust and would like to replace that with ICU4X. In case of a successful test262 April test, we’ll be in a good position to offer Fuchsia the ability to test a replacement of the same subset of APIs backed by ICU4C to ICU4X.
Copy link
Contributor

Choose a reason for hiding this comment

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

related to 262 full compatibility requirement above.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I'm confused. Are you concerned that a reader may interpret "full test262 compatibility" as "producing JS objects" rather than "performing i18n operations that are in scope of ecma402 and tested by test262"?

Do you have a suggestion on how we should communicate that intent?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please commit as is, I'll edit after.

Copy link
Contributor

@dminor dminor left a comment

Choose a reason for hiding this comment

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

This is looking good, my comments are mostly grammar nits.

docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/prd.md Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/prd.md Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
@sffc sffc self-requested a review April 21, 2021 14:11
sffc
sffc previously approved these changes Apr 21, 2021
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.

LGTM from my end, modulo my suggestions.

In general, this is written largely from the Mozilla perspective (as expected). After this is checked in, I may want to do a pass where I add more content from the Google perspective.

docs/process/roadmap.md Show resolved Hide resolved
docs/process/roadmap.md Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
Manishearth
Manishearth previously approved these changes Apr 21, 2021
Copy link
Contributor

@nciric nciric left a comment

Choose a reason for hiding this comment

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

I'll edit test262 section(s) and work with you on the review.

echeran
echeran previously approved these changes Apr 22, 2021
Copy link
Contributor

@echeran echeran left a comment

Choose a reason for hiding this comment

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

LGTM


### Collator

Collation is one of the core [ECMA-402 components](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/Collator), and builds on top of the Unicode Properties API which we’re investing in.
Copy link
Contributor

Choose a reason for hiding this comment

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

question (non-blocking): Doesn't collation also depend on normalization? If so, it would be worth mentioning, and that would also be another reason why collation isn't a quick/easy thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

@sffc ^ do you know? I have never worked on collation impl.

Copy link
Member

Choose a reason for hiding this comment

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

I don't feel authoritative answering this question since I have not worked extensively with collation.

docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/prd.md Outdated Show resolved Hide resolved
docs/process/roadmap.md Outdated Show resolved Hide resolved
docs/process/roadmap.md Outdated Show resolved Hide resolved
nciric
nciric previously approved these changes Apr 22, 2021
dminor
dminor previously approved these changes Apr 22, 2021
sffc
sffc previously approved these changes Apr 22, 2021
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.

Suggested some new issue numbers; otherwise LGTM

docs/process/roadmap.md Outdated Show resolved Hide resolved
docs/process/roadmap.md Outdated Show resolved Hide resolved
@zbraniecki zbraniecki merged commit 62d5416 into unicode-org:main Apr 26, 2021
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.

Add roadmap
9 participants