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

Convertinging util.js to typescript 3/3 #39

Merged
merged 4 commits into from
Feb 25, 2022
Merged

Conversation

waylandli
Copy link

@waylandli waylandli commented Dec 11, 2021

Problem

Github Issue #28
Part 3 of 3
Converting util.js to typescript

Change summary:

  • Converted util.js to typescript

Steps to Verify:

  • Tests should still run while the file is in typescript

@waylandli waylandli changed the title wip Convertinging util.js to typescript 3/3 Dec 11, 2021
lib/util.ts Outdated Show resolved Hide resolved
lib/util.ts Outdated Show resolved Hide resolved
lib/util.ts Outdated Show resolved Hide resolved
@waylandli
Copy link
Author

A lot of //@ts-ignore used because a lot of imported classes don't contain typings for their functions which causes typescript errors.

@waylandli waylandli marked this pull request as ready for review February 24, 2022 18:38
Copy link
Collaborator

@shannonwells shannonwells left a comment

Choose a reason for hiding this comment

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

Approved assuming there's some plan for removing all the @ts-ignores.

@@ -27,29 +35,34 @@ class fixedTFramedTransport extends thrift.TFramedTransport {
*/

const previousPageLocation = parquet_thrift.PageLocation.prototype;
//@ts-ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

I haven't been following along on these conversions - are we just using @ts-ignore for now? Is there a plan to convert these to proper TypeScript? @wilwade

Copy link
Member

@wilwade wilwade left a comment

Choose a reason for hiding this comment

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

💯

@waylandli waylandli merged commit 14bbb97 into main Feb 25, 2022
@waylandli waylandli deleted the chore/util-conversion/28 branch February 25, 2022 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants