Skip to content
This repository has been archived by the owner on Sep 30, 2019. It is now read-only.

Event bus #31

Merged
merged 8 commits into from
Aug 10, 2016
Merged

Event bus #31

merged 8 commits into from
Aug 10, 2016

Conversation

dennari
Copy link
Contributor

@dennari dennari commented Aug 9, 2016

The design of the Event<T> interface is based on the discussion in here: reduxjs/redux#992.

it('should match localhost hostname with single-digit ids', () => {
ret = getDeploymentKey('fdlkasjs-4-1.localhost') as DeploymentKey;
const ret = getDeploymentKey('fdlkasjs-4-1.localhost');
if (ret === null) { throw new Error(); }
Copy link
Contributor

Choose a reason for hiding this comment

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

Would not expect(ret).to.exist be cleaner?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but this way TypeScript knows to narrow down DeploymentKey | null without explicit casting. The error message could be a bit more descriptive, though :D .

@juhoojala
Copy link
Contributor

Except the two comments looks good.

@juhoojala juhoojala merged commit 53413c1 into master Aug 10, 2016
@juhoojala juhoojala deleted the event-bus branch August 10, 2016 19:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
2 participants