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

fix(types): Copy types from DefinitelyTyped #283

Merged
merged 1 commit into from Aug 19, 2021
Merged

fix(types): Copy types from DefinitelyTyped #283

merged 1 commit into from Aug 19, 2021

Conversation

kachkaev
Copy link
Contributor

@kachkaev kachkaev commented Aug 19, 2021

This PR partially addresses #191 (see #191 (comment) → step 1). The file was copied from DefinitelyTyped.

@kachkaev
Copy link
Contributor Author

kachkaev commented Aug 19, 2021

Let me check the changes locally before removing the draft status. If anyone spot anything in the meantime, feel free to raise it in the comments 🙂

I tried to keep the file as close to the original to avoid accidental breaking changes.

UPD: I made the same changes locally and the typings were successfully read. Removing the draft status.

@kachkaev kachkaev marked this pull request as ready for review August 19, 2021 21:10
Copy link
Member

@karfau karfau left a comment

Choose a reason for hiding this comment

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

It's a pity we are loosing the tests that are present in DefinitlyTyped.
We should add them as before we make any relevant changes.

Besides that: LGTM

@kachkaev
Copy link
Contributor Author

kachkaev commented Aug 19, 2021

cc @tkqubo, @karfau (you are mentioned in DefinitelyTyped → index.d.ts) 👋

UPD Oops I forgot to call a string comparison function in my head before cc-ing @karfau 😂

@kachkaev
Copy link
Contributor Author

Yeah I completely agree on the tests. This fix we are making is quite rough, but it looks justified given the state of urgency 🙂

@karfau
Copy link
Member

karfau commented Aug 19, 2021

Need to get some sleep now, but will release it soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Pull requests that update a dependency file types Anything regarding Typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants