search

whitlockjc / path-loader

Utility that provides a single API for loading the content of a path/URL.
MIT License
8 stars 18 forks source link

fix the api error #9

Closed iyuq closed 8 years ago

iyuq commented 8 years ago

fix the api error, it should pass a null object to the callback as the first arguments

whitlockjc commented 8 years ago

This is not the appropriate way to go about fixing this. docs/API.md is generated from the sources so you'd want to update the appropriate source to fix this. Then you'd run gulp to lint check the code, test your changes and generate the binaries/documentation. Update your PR appropriately and we'll get it merged.

iyuq commented 8 years ago

@whitlockjc I changed this by change the comments in the code, is this okay?

whitlockjc commented 8 years ago

That's right but I would expect the generated files in browser/* to be updated as well. Changing index.js was the right place to fix it so run gulp, add all files to the commit (including the browser/*.js files) and update the PR. Also, do you mind squashing your commits into one commit?

iyuq commented 8 years ago

@whitlockjc It doesn't matter to squashing my commits into one. Is this all right?

whitlockjc commented 8 years ago

I would rather you squash the commits because it makes for an easier to understand Git history. As for the current changes, they are accurate.

iyuq commented 8 years ago

@whitlockjc I squash the commits into one in 4bbfcb52e5e7cb77ab86b570aa9492546420f3a0.

iyuq commented 8 years ago

@whitlockjc I reopened a pull request to report the pull request