Skip to content

Conversation

@orta
Copy link
Contributor

@orta orta commented Aug 30, 2021

Re: #45646 - no breaking changes in here though

This is quite a few PRs + issues combined, rebased etc:

I've tried to reduce the overall character count for some of the docs (e.g. removing some JSDocs which are redundant because the type system will tell you the types of things) and removed all the specs links, I can't imagine your average Intl API consumer wanting to read the spec (over MDN).

Also adds some fresh docs about how lib files work to the README, covering how Intl is a wee bit special in comparison to some of the other cases we have in the .d.ts files.

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Aug 30, 2021
@orta
Copy link
Contributor Author

orta commented Aug 31, 2021

@typescript-bot run dt

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 31, 2021

Heya @orta, I've started to run the parallelized Definitely Typed test suite on this PR at 5f67f65. You can monitor the build here.

@orta
Copy link
Contributor Author

orta commented Aug 31, 2021

I've double checked all the es5 Intl properties, and they are in the right place (they were spec'd in 2012 which is a reasonable approximation IMO)

@orta
Copy link
Contributor Author

orta commented Aug 31, 2021

@longlho any chance I could get your eyes over this? (and sorry it's taken a while!)

@longlho
Copy link
Contributor

longlho commented Aug 31, 2021

Yup will take a look asap

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

Just a lot of minor edit suggestions.

Comment on lines 220 to 223
hourCycle?: "h11" | "h12" | "h23" | "h24" | undefined;
}

interface DateTimeFormatOptions {
Copy link
Member

Choose a reason for hiding this comment

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

Probably should merge these two DateTimeFormatOptions declarations

Copy link
Contributor

@longlho longlho left a comment

Choose a reason for hiding this comment

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

Overall I think es2021 is missing a ton of stuff but I think it's a good enough set of changes in the scaffolding for others to fill in.

@orta
Copy link
Contributor Author

orta commented Sep 6, 2021

OK, thanks for the feedback everyone - assuming this is green I will merge weds and start closing all the linked PRs

@longlho
Copy link
Contributor

longlho commented Sep 6, 2021

Editorial suggestions but overall lgtm

@orta
Copy link
Contributor Author

orta commented Sep 8, 2021

Thanks folks

@orta orta merged commit 07fd7bc into microsoft:main Sep 8, 2021
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants