This one is a doozy of a PR #85
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added standard as a dev dependency and ran the command `standard --fix`, which automatically fixes indentation, tabs, spacing, equality operators, etc.
Most development is done using build tools such as webpack, so maintaining a build of the package in this form is redundant. It doesn't appear to be maintained much either, so this will make the package a little more leaner
I think this was supposed to be removed since it's in the ignore files, but was still in source control
It seemed pointless to call a script that called a script. It could be called from package.json just as well
Since json gets converted to normal js when you require it, there was really no reason to have it as json other than it's inconvenient to add new stuff to it.
…nd moved to the index file Sorry this couldn't be more atomic. This is a big one... The module is now an es6 class. Instead of using callbacks, promises are implemented, which will fix #64. Limit.js is also used which was talked about in PR #74. Instead of using the index file to just point to the class in the lib, move the class into the index file. It seemed a little redundant to me.
The timeout had to be increased to accomodate the throttle time (10 seconds)
Some endpoints (like watchlist endpoints) have an optional id parameter. So by default, if there's a session id and the endpoint has an id parameter and none was passed to the function, it will use the default {account_id} as the id parameter. Cleaned up the tests since they all called the same assertions. And added a test for this default id paramter.
If the endpoint has one placeholder and there's a non object argument, assume it's meant for the endpoint
|
Any plans to merge this PR? |
|
Any update on this? If you don't want to merge it (understandable), then I can just make it into a separate package and reference this one |
|
I went ahead and published moviedb-promise. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This completely refactors the module and makes a lot of breaking changes (for the better I hope). Definitely a hefty PR.
Looking forward to the discussion 👍