-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Use node ecosystem #20
Comments
I get that node users prefer this flow, but what would be the benefit? FWIW, browserify already works since its just a polyfill, not a module with any exported symbols. |
If anything, going in the opposite direction would be preferred to me. We can remove the |
How does the test tooling this repo uses affect you're actual application integration of the polyfill? |
Maybe you're confused, this polyfill is for running in browser environments, not node. Which is why we want to test in PhantomJS. |
Honestly, my suggestions were mainly out of the fact that this is a JS project, that already needs node (for bower, phantom-js, your test environment, etc) and could, via some minor adjustments use that same environment without any need for additional tooling, and work cross platform. Windows and OSX don't have I'm now using |
I'm going to close this out since it's working well so far. I appreciate the suggestion to use more node ecosystem tools, but we can rely on |
Since you are already publishing to Bower, which requires node, I would suggest the following adjustments.
request
Makefile
with npm tasks that run from package.json (example below)Example
package.json
for scriptsnpm install
will follow up with
bower install
(npm will resolve the execution path from node_modules)npm test
will run linting and your existing test script
npm run build
will clean and re-install
I added the
rimraf
to remove the dependency.Note, if you're open to pull requests, I'd be happy to do the initial changes to remove the dependencies on tools outside of node (namely the makefile, and shell script)
The text was updated successfully, but these errors were encountered: