-
Notifications
You must be signed in to change notification settings - Fork 212
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
refactor: remove all unnecessary @ts-ignore annotations #2441
Conversation
0af0d75
to
5ad04cb
Compare
The |
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.
Please keep the @ts-ignore
above t.throws
and t.throwsAsync
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'd like to understand better why some of these are no longer necessary. Not critical.
I'm not sure about the Zoe stuff. I guess I'll stand by for Kate's comments... but that doesn't seem like an algorithm that terminates. Splitting the PR would make for a more clear process, but maybe that's more trouble than it's worth.
@@ -1,4 +1,3 @@ | |||
// @ts-ignore |
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 don't understand why this is no longer necessary. hm.
Sorry, I had assumed those files were running with Would you like me to add |
That would be great, thanks! |
@katelynsills would you be able to take a look at the documentation snippets failure at: https://github.com/Agoric/agoric-sdk/pull/2441/checks?check_run_id=1913294837#step:12:10 Thanks! |
Yup! |
5ad04cb
to
ab2c765
Compare
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.
LGTM! Thanks for doing this cleanup
ab2c765
to
0fd4cd7
Compare
Is this waiting on something? The documentation checks are passing 👍 |
0fd4cd7
to
10afb9c
Compare
Not waiting on anything now! Thanks for the prod. |
Either updates to the Typescript version we use in agoric-sdk, or cargo-culting means many of our
@ts-ignore
annotations are unnecessary.I suggest that we should avoid
@ts-ignore
whenever we can: only use it when Typescript really has a bug we can't work around, such as #2438 (comment). Then, link to the corresponding open issue on the TypeScript repository in the same line as@ts-ignore
.