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

several ts-nocheck resolved #1193

Merged
merged 4 commits into from
Oct 30, 2021
Merged

several ts-nocheck resolved #1193

merged 4 commits into from
Oct 30, 2021

Conversation

rvilarl
Copy link
Collaborator

@rvilarl rvilarl commented Oct 23, 2021

There are only two ts-nocheckpending to be resolved:

One in textnote_tests.ts @ronyeh could you include in #1163?
Another one in typeguard_tests.ts which I do not know how to resolve.

@rvilarl
Copy link
Collaborator Author

rvilarl commented Oct 23, 2021

Ready to be reviewed.

from: Note;
to: Note;
from: Note | null;
to: Note | null;
Copy link
Contributor

@sschmidTU sschmidTU Oct 23, 2021

Choose a reason for hiding this comment

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

Just curious about having null and not undefined. I really wished there was only one thing for that in TS, not two. Though it seemed to me like null is discouraged in Typescript.
At the end of the file changes list, e.g. in stavetie_tests.ts, there was also a comment about maybe changing null to undefined.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For API compatibility. No other reason. I understood that 4.0 has enough changes already.
@ronyeh @0xfe Thought?

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, that's right.

@0xfe
Copy link
Owner

0xfe commented Oct 24, 2021

Can you fix the conflicts please?

@rvilarl
Copy link
Collaborator Author

rvilarl commented Oct 24, 2021

@0xfe fixed, ready to review

@rvilarl rvilarl mentioned this pull request Oct 24, 2021
@0xfe
Copy link
Owner

0xfe commented Oct 30, 2021

Looks good. Thanks Rodrigo.

@0xfe 0xfe merged commit 6e28ebb into 0xfe:master Oct 30, 2021
@rvilarl rvilarl deleted the fix/ts-nocheck branch December 27, 2022 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants