-
Notifications
You must be signed in to change notification settings - Fork 440
Update README to match the new code #1064
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
Conversation
This should match better with the code after microsoft#1034.
Thanks for the PR! This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not too sure about the removal of heuristics, which still somewhat apply. @orta can you take a look too?
|
||
### What are the TypeScript team's heuristics for PRs to the DOM APIs | ||
|
||
Changes to this repo can have pretty drastic ecosystem effects, because these types are included by default in TypeScript. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This paragraph is mostly still true, right? I know we are less conservative than before, but the idea is that every update to @types/web
will eventually ship with Typescript by default means that there are some changes that will not be OK until there's a way to publish other packages from here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, maybe it's good to keep the sentence for maintainers. It has minimal meaning for contributors though, since nearly all changes will now be from webref and MDN rather than manual PRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know we are less conservative than before
I'm not sure, as quite a lot types are removed for lack of broad browser support?
Co-authored-by: Nathan Shively-Sanders <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, yeah, I think this probably what we want
Cool, I'd like to merge this if @sandersn also agrees. |
LGTM |
Merging because @saschanaz is a code-owner of all the changes - thanks! |
This should match better with the code after #1034.