Skip to content

Fix up packaging #75

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

Merged
merged 3 commits into from May 20, 2016
Merged

Fix up packaging #75

merged 3 commits into from May 20, 2016

Conversation

ghost
Copy link

@ghost ghost commented Apr 20, 2016

Purpose

Clean up packaging. Several commits have been lost due to the confusing way this package is structured.

Changes

This PR adds Babel primarily to get UMD bundling. This enables the module to be consumed by any environment transparently falling back to using the window if needed. This removes the shell scripts in favor of npm scripts as well.

I have also recommitted d92fe86 since it was blown away by the old build process.

SemVer Changes

This will require a major version bump (to 2.0) because of the breaking changes

Closes #74

import angular from 'angular';
import * as ws from 'ws';

var Socket = (ws.Client || ws.client || ws);
Copy link
Author

Choose a reason for hiding this comment

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

This check previously existed inside of a function call. It is preferable to do these kinds of checks outside of any particular function. Also, the try/catch block that silently swallowed errors was a bit of a code smell to me. UMD fixes the problem (I think) you were trying to solve via that mechanism.

Copy link
Author

Choose a reason for hiding this comment

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

Tests fail when this is removed. So I put it back in.

Will Soto added 3 commits April 20, 2016 14:18
This recommits d92fe86

Signed-off-by: Will Soto <will.soto@fanduel.com>
Signed-off-by: Will Soto <will.soto@fanduel.com>
@PatrickJS
Copy link
Owner

LGTM

@PatrickJS PatrickJS merged commit 91818ea into PatrickJS:master May 20, 2016
@b264 b264 mentioned this pull request May 21, 2016
@ghost ghost deleted the packaging branch July 7, 2016 12:57
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.

1 participant