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

Update index.js #4

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

BlackFenix2
Copy link
Collaborator

Quick update to fix the package crashing as mentioned in #3

Quick update to fix the package crashing as mentioned in leadstoloyals#3
@christiaanwesterbeek
Copy link
Member

I'm not sure why you get the error, because let x = new BigNumber(123.4567) (or BigNumber(123.4567)) is supposed to be the way to create the number according to the docs.

It's probably related to a combination of the way the Bignumber repo was setup and your js-bundler. More people reported the problem.

I'm not sure if your fix is introducing a problem with other bundlers...

@BlackFenix2
Copy link
Collaborator Author

@christiaanwesterbeek I noticed you dont have a LICENSE.MD in your repo, can i fork this project and just work off that? i have to use async/await in my workspace.

@christiaanwesterbeek
Copy link
Member

I'm fine with that. We can also consider to bring you modernisations back to this repo afterwards and create a 2.x version if it's incompatible with the current 1.x.

@BlackFenix2
Copy link
Collaborator Author

@christiaanwesterbeek yeah, this project uses the depreciated request NPM package. i can modernize by switching to axios for http requests.

@BlackFenix2
Copy link
Collaborator Author

@christiaanwesterbeek i completed my PR and added a createGiftCardAsync() method to allow promise-based API access through axios.

it should not require a major version change since i just added an API endpoint without breaking the existing API. let me know iif you need anything else.

@christiaanwesterbeek
Copy link
Member

It looks backward compatible, and if you had the same experience. I think I can merge this one in and release a 1.1 on npm which is a minor version increment.

@BlackFenix2
Copy link
Collaborator Author

@christiaanwesterbeek sounds good, when can we publish the minor version to NPM?

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