search

johntitus / coinx

Buy, sell, and find the best prices for crypto-currencies from multiple markets.
MIT License
151 stars 37 forks source link

Configuration rewrite #19

Closed alexandrfox closed 6 years ago

alexandrfox commented 6 years ago

Hi,

Here's a rewrite of the configuration part. coinx-core.js is supposed to be a central place of the app now, providing modules with available/configured exchanges and coins so there's no need to change all modules in case new market is added. I guess usage of static methods there is less than ideal, but most likely still better than hardcode.

P.S. Resubmitted PR with indentation matching your style.

johntitus commented 6 years ago

This looks like a GREAT improvement! Thank you very much. I need to take some more time with it but plan to merge in the next day or so.

On Sat, Jul 15, 2017 at 8:51 AM, Alex Mironov notifications@github.com wrote:

Hi,

Here's a rewrite of the configuration part. coinx-core.js is supposed to be a central place of the app now, providing modules with available/configured exchanges and coins so there's no need to change all modules in case new market is added. I guess usage of static methods there is less than ideal, but most likely still better than hardcode.

P.S. Resubmitted PR with indentation matching your style.

You can view, comment on, or merge this pull request online at:

https://github.com/johntitus/coinx/pull/19 Commit Summary

  • Configuration rewrite: new API can be added without any changes

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/johntitus/coinx/pull/19, or mute the thread https://github.com/notifications/unsubscribe-auth/AAkB_tg3i1_sS9gpEs_CjqXFvpAdWnG4ks5sOLXFgaJpZM4OZAgI .