Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

shapeshift Promise API library #2088

Merged
merged 6 commits into from
Sep 15, 2016
Merged

shapeshift Promise API library #2088

merged 6 commits into from
Sep 15, 2016

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Sep 14, 2016

Add a ShapeShift Promise-based API library for the upcoming fund integration into the UI. (Same location as the etherscan item)

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. M-UI labels Sep 14, 2016
@parity-cla-bot
Copy link

It looks like @jacogr signed our Contributor License Agreement. 👍

Many thanks,

Ethcore CLA Bot

@derhuerst
Copy link
Contributor

Why don't we use chovy/shapeshift or ExodusMovement/shapeshift.io?

@jacogr
Copy link
Contributor Author

jacogr commented Sep 14, 2016

The reality is that this code actually precedes the existence of both those. In this case I prefer a home-grown solution

  • don't need all the endpoints for our use-case
  • one less external dep that we need to pull in (same deps as our other stuff)

As for specifics to those libraries -

  • chovy - doesn't support API keys (affiliate), nor does it actually cater for the CORS endpoint, does have Promises, which is a positive
  • Exodus - not Promised-based, will need to wrap, ApiKey not available for shift (interesting can get the txlist for it), does cater for CORS, which is a positive

@derhuerst
Copy link
Contributor

one less external dep that we need to pull in (same deps as our other stuff)

But more code to maintain & more general "mental overload" because of a huge project.

Exodus - not Promised-based, will need to wrap, ApiKey not available for shift (interesting can get the txlist for it), does cater for CORS, which is a positive

I think a PR that builds on top of ExodusMovement/shapeshift.io#3 would be nice here. Let's contribute back & not lock everything into one giant repo. We'd be able to comfortably use the lib if someone else had bothered to contribute.

@jacogr
Copy link
Contributor Author

jacogr commented Sep 14, 2016

Rather -

  1. I know this library works, build ShapeShift integration on top of it already, ready to import from a UI perspective
  2. I'd rather spend the 3 hours importing (UI-wise) what I know is working and tested on the live network than working on another repo fixing another lib, waiting for the PR to get merged at some point and then re-developing what is there from a UI perspective

Quite happy to re-visit at some point if need be, but at this point I see no gains whatsoever, just a long draw-out process for 3 API calls.

EDIT: And Exsodus is the wrong one, not Promised-based - not going to add PR for wrappers on top of it.

@ngotchac ngotchac merged commit 186baff into js Sep 15, 2016
@jacogr jacogr deleted the js-3rdparty-shapeshift branch September 16, 2016 13:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-pleasereview 🤓 Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants