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

Improve NPM experience #33 #34

Merged
merged 2 commits into from
Nov 10, 2017
Merged

Improve NPM experience #33 #34

merged 2 commits into from
Nov 10, 2017

Conversation

Meligy
Copy link
Contributor

@Meligy Meligy commented Nov 6, 2017

Fixes #33

@Meligy
Copy link
Contributor Author

Meligy commented Nov 6, 2017

Can we please publish this if it's OK to merge?

Cheers,

@Meligy Meligy mentioned this pull request Nov 8, 2017
@Meligy
Copy link
Contributor Author

Meligy commented Nov 9, 2017

Good point about tests. They were added by mistake because I used a tool to generate the file.

I'll remove them and update the PR.

Regarding Node, let's include it the way it is for now. You already have good naming for them.

Ideally you'd want a neater import for these like import * as appAuthNode from '@openid/appauth/node_support';, but I don't know how to do this yet, and it seems not easy when you don't compile your TS files to the same folder, judging by microsoft/TypeScript#8305 (comment).

So what we have will work for now for Node.

@Meligy
Copy link
Contributor Author

Meligy commented Nov 9, 2017

Check it now and let me know what you think. I deleted the tests references from the barrel.

Meligy referenced this pull request in Meligy/AppAuth-JS Nov 9, 2017
@tikurahul
Copy link
Collaborator

tikurahul commented Nov 9, 2017

Okay, this looks better. Can you also sign the CLA ?

https://github.com/openid/AppAuth-JS/blob/master/CONTRIBUTING.md

@Meligy
Copy link
Contributor Author

Meligy commented Nov 9, 2017

Done for both 2 CLA documents.

@Meligy
Copy link
Contributor Author

Meligy commented Nov 9, 2017

Also received an email from Mike Leszcz saying the signature was processed.

@tikurahul tikurahul merged commit 48f641c into openid:master Nov 10, 2017
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