search

interledger-deprecated / ilp-core

Core ILP module — handles ledger abstraction and quoting
Other
8 stars 5 forks source link

remove extensions #35

Closed emschwartz closed 8 years ago

emschwartz commented 8 years ago

oops -- another breaking change. let's wait to bump the version with this one until we have another set of breaking features to add. this removes something that isn't being used right now so it can stay in until the next major release

resolves https://github.com/interledger/js-ilp-core/issues/33

codecov-io commented 8 years ago

Current coverage is 98.03% (diff: 100%)

Merging #35 into master will decrease coverage by 0.10%

@@             master        #35   diff @@
==========================================
  Files             4          4          
  Lines           162        153     -9   
  Methods          28         27     -1   
  Messages          0          0          
  Branches         38         36     -2   
==========================================
- Hits            159        150     -9   
  Misses            3          3          
  Partials          0          0

Powered by Codecov. Last update 0bb03f7...6b716cc

justmoon commented 8 years ago

Can you explain why this feature was removed? Do we not need extensibility or is there a better way to do it?

emschwartz commented 8 years ago

@justmoon This extension mechanism provides no benefit over instantiating a client and passing it to the Extension constructor. I originally thought this would be a good way to manage extensions, but the main thing I thought would be an extension -- what has become js-ilp -- ended up using the functionality of the core client simply by instantiating one, using it, and exposing a different API.

justmoon commented 8 years ago

Makes sense, LGTM