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

CommonJS package manager support #129

Merged

Conversation

flaviogranero
Copy link
Contributor

I'm trying to use ng-token-auth in a project that manages its modules using browserify. In order to be able to load this lib as a CommonJS module, we need a few changes I'm including in this pull request.

Let me know if you have any questions.

@lynndylanhurley
Copy link
Owner

Hi @flaviogranero - thank you for sending this PR!

I'm all about merging this, but I'm having a hard time reviewing the changes because every single line is changed (I'm sure it's due to indentation).

Before I go through it line by line, is there a way to make a version of this PR that shows only the changes?

Thanks again!

@flaviogranero
Copy link
Contributor Author

hi @lynndylanhurley, yes.. I was trying to put all the code inside a IIFE to make the code scope more secure, but sounds like git is not showing a good diff, so I removed this part of the change.

@lynndylanhurley
Copy link
Owner

Cool, thx @flaviogranero! If you push the IIFE as a separate PR, I'll merge that as well.

lynndylanhurley added a commit that referenced this pull request Mar 16, 2015
@lynndylanhurley lynndylanhurley merged commit 462e756 into lynndylanhurley:master Mar 16, 2015
@lynndylanhurley
Copy link
Owner

And welcome new contributor 🎱

@flaviogranero
Copy link
Contributor Author

😄

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