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

remove backwards compatible code, use npm versioning instead #3

Merged
merged 1 commit into from
Jun 17, 2014
Merged

remove backwards compatible code, use npm versioning instead #3

merged 1 commit into from
Jun 17, 2014

Conversation

ralphtheninja
Copy link
Contributor

Removed code for backwards compatibility. If you publish this as 0.2.0, code that needs to be backwards compatible can still use 0.1.0. Let npm and package.json take care of the rest :)

Note that I also changed the return value to an empty object instead of returning null if no info can be found for a particular coin.

I aim to add more support for default ports (could also have default rpc port) and also for network magic numbers.

@jprichardson
Copy link
Member

Hmm, not sure how I missed this PR. Sorry. Why do you think {} is better than null?

@ralphtheninja
Copy link
Contributor Author

I prefer to avoid null if possible and instead check the property on the returned object. I can put back null if you want.

@jprichardson
Copy link
Member

Yeah, I'd prefer null. It seems odd to return a truthy value for something that doesn't exist.

This feels pretty intuitive:

if (!coininfo('FAKE-COIN'))
  console.log('FAKE-COIN does not exist')

@ralphtheninja
Copy link
Contributor Author

Updated.

jprichardson added a commit that referenced this pull request Jun 17, 2014
remove backwards compatible code, use npm versioning instead
@jprichardson jprichardson merged commit 11b44c8 into cryptocoinjs:master Jun 17, 2014
@jprichardson
Copy link
Member

Sorry this took me so long to review!! Thanks again :)

@ralphtheninja
Copy link
Contributor Author

Np :)

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