Skip to content
This repository was archived by the owner on Jun 17, 2019. It is now read-only.

Conversation

@sean-hill
Copy link

No description provided.

@hpoom
Copy link
Contributor

hpoom commented Jun 21, 2016

I don't think this is the best solution. The issue I see is that 1 gets passed in to select provider 1 but on line 90 it increments providers when it fails, but this would not work if provider was 1 and we would loose this backup. Also there is no protection or range checking to stop somebody passing in -1 or 99 as a value.

PR 33 is about to change this as well to make calls to all providers happen and fastest one win. I suggest best route forward would be to wait for #33 to be merged then we can look at a solution to turn on and off providers.

I still feel passing in a config would be best. So that you can override all of providers.js and add in new custom providers not even covered by this NPM module.

@hpoom
Copy link
Contributor

hpoom commented Jun 21, 2016

@jamescogley have I overlooked anything here? What are your thoughts on the best way to achieve what @sean-hill needs?

@sean-hill
Copy link
Author

You guys are the best :)

@jamescogley
Copy link
Contributor

@hpoom I agree, passing the providers in as config would be the best option, it would give the most flexibility

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants