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(daemon): Strengthen type checks #1832

Merged
merged 8 commits into from
Oct 17, 2023
Merged

Conversation

kriskowal
Copy link
Member

Evidently type checks have drifted because @ts-check comments are for some reason not redundant with "checkJs": true as advertised. This change gets ts-check to pass for all the daemon sources.

@kriskowal kriskowal requested a review from kumavis October 12, 2023 22:24
@kriskowal kriskowal force-pushed the kriskowal-endo-types-baseline branch 2 times, most recently from 535063a to 12835ec Compare October 13, 2023 02:43
@kriskowal kriskowal force-pushed the kriskowal-endo-types-baseline branch from 12835ec to 14d1abc Compare October 16, 2023 22:06
Comment on lines +50 to +51
acks: AsyncSpring<IteratorResult<TRead, TReadReturn>>,
data: AsyncSink<IteratorResult<TWrite, TWriteReturn>>,
Copy link
Member Author

Choose a reason for hiding this comment

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

I imagine @mhofman will be interested in this relaxation of the signature of makeStream. This is a remnant of the pubsub work. Although we’re not pursuing that variety of pubsub, this does make the type signature of makeStream no more specific than necessary, which I think is good POLA.

Copy link
Member

@kumavis kumavis left a comment

Choose a reason for hiding this comment

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

🎉

/**
* @param {object} args
* @param {string} args.path
* @param {Promise<unknown>} args.cancelled
Copy link
Member

Choose a reason for hiding this comment

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

inconsistent types (see comment above)
here Promise<unknown>

* @param {object} args
* @param {number} args.port
* @param {string} [args.host]
* @param {Promise<never>} args.cancelled
Copy link
Member

Choose a reason for hiding this comment

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

inconsistent types (see comment below)
here Promise<never>

@kriskowal kriskowal force-pushed the kriskowal-endo-types-baseline branch from 14d1abc to 2f8aa51 Compare October 17, 2023 18:15
@kriskowal kriskowal merged commit 58e8d06 into endo Oct 17, 2023
14 checks passed
@kriskowal kriskowal deleted the kriskowal-endo-types-baseline branch October 17, 2023 18:16
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.

2 participants