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

Refining types for waiter-manager #11

Merged
merged 2 commits into from May 28, 2019
Merged

Refining types for waiter-manager #11

merged 2 commits into from May 28, 2019

Conversation

scalvert
Copy link
Collaborator

The types in waiter-manager.ts were a little loose. This PR tightens them up a bit.

@scalvert scalvert requested a review from rwjblue May 24, 2019 00:10
@@ -4,7 +4,7 @@ export type Token = unknown;
export interface IWaiter {
name: WaiterName;
waitUntil(): boolean;
debugInfo(): unknown;
debugInfo(): ITestWaiterDebugInfo[];
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather the generic thing not be forced into the same structure as what TestWaiter would use.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought about this too, but from a consumer standpoint, we expect to output this debug info as part of @ember/test-helpers. Specifically, that library does expect this to be a particular format. In fact, it exactly expects this format, as demonstrated here. How would we handle this otherwise?

Copy link
Member

Choose a reason for hiding this comment

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

Kk, seems good then. Thank you!

@scalvert scalvert merged commit ea5a354 into master May 28, 2019
@scalvert scalvert deleted the refining-types branch May 28, 2019 02:33
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.

None yet

2 participants