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

Add isPromise #61

Merged
merged 1 commit into from
Nov 15, 2016
Merged

Add isPromise #61

merged 1 commit into from
Nov 15, 2016

Conversation

SimenB
Copy link
Contributor

@SimenB SimenB commented Nov 3, 2016

Ref #39

/cc @sveisvei

@@ -257,6 +257,8 @@ isOpen|N/A|boolean|Returns `true` if circuit is open
- **healthCheckInterval**: time in `ms` interval between each execution of health check function
- **healthCheck**: function to call for the health check (can be defined also with calling `healthCheck` function)
- **fallback**: function to call for fallback (can be defined also with calling `fallback` function)
- **isPromise**: `boolean` to opt out of check for callback in function
Copy link
Owner

@awolden awolden Nov 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add something in here telling the user that the flag will also be applied to thehealthCheck and fallback. I don't think that will be obvious to an end-user.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@sveisvei
Copy link

sveisvei commented Nov 5, 2016

Lgtm.

Might suggest we export a new PromiseBrakes() that defaults to isPromise:true but not sure it will just be confusing, up 2 you @awolden .

@SimenB
Copy link
Contributor Author

SimenB commented Nov 10, 2016

@awolden ping 😄

@awolden
Copy link
Owner

awolden commented Nov 15, 2016

@sveisvei as these idiosyncrasies show up in the library it lends itself to rethinking how the API for brakes is designed, and it is good food for though regarding a possible version 3. For now, I think this change is satisfactory as is.

@awolden awolden merged commit 3f7728a into awolden:master Nov 15, 2016
@SimenB SimenB deleted the is-promise branch November 15, 2016 21:58
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.

None yet

3 participants