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

Add new entry point: index.js #84

Merged
merged 1 commit into from May 4, 2016
Merged

Add new entry point: index.js #84

merged 1 commit into from May 4, 2016

Conversation

fanatid
Copy link
Member

@fanatid fanatid commented Apr 29, 2016

I think that try-catch in bindings.js is wrong way.

@axic
Copy link
Contributor

axic commented Apr 29, 2016

Why do you need a try/catch? Why not just use bindings.js as the entry point and override it for browserify?

@fanatid
Copy link
Member Author

fanatid commented Apr 29, 2016

Original issue: #71
try-catch is helpful when compilation fail..

@axic
Copy link
Contributor

axic commented Apr 29, 2016

I assumed if compilation fails, the package fails to install?

@fanatid
Copy link
Member Author

fanatid commented Apr 29, 2016

Not at all, if you put package that require compilation to optionalDependencies and if compilation fails, exit code will be still 0..

$ cat package.json
{
  "dependencies": {
    "elliptic": "^6.2.3"
  },
  "optionalDependencies": {
    "secp256k1": "^3.0.0"
  }
}
$ CXX=g-- npm i
...
make: g--: Command not found
...
gyp ERR! build error
...
$ echo $?
0

@axic
Copy link
Contributor

axic commented May 2, 2016

@fanatid I think its a bit nonstandard behaviour to marry the fact optionalDependencies still leaves the package if compilation failed.

If it works, it works. Would be nice to eventually have this as a proper feature in package.json or gyp.

@fanatid fanatid merged commit 3eb777e into master May 4, 2016
@fanatid fanatid deleted the feature/new-entry-point branch May 4, 2016 17:12
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