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

#266 improve the spec to take in explicit trues #270

Closed
wants to merge 1 commit into from

Conversation

Emily-Jiang
Copy link
Member

Signed-off-by: Emily Jiang [email protected]

@Emily-Jiang
Copy link
Member Author

just spec change..... tck changes will come soon.

Fault Tolerance policies can be disabled with configuration at method level, class level or globally for all deployment.
Fault Tolerance policies can be disabled with configuration at method level, class level or globally for all deployment. If multiple configurations are specified, method-level configuration overrides class-level configuration, which then overrides global configuration. e.g.
* `com.acme.test.MyClient/methodA/CircuitBreaker/enabled=true`
* `com.acme.test.MyClient/CircuitBreaker/enabled=false`
Copy link
Member

Choose a reason for hiding this comment

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

This middle line doesn't make a lot of sense. It's valid but has no effect because CircuitBreaker is disabled globally.

I think the example would be clearer if this line was removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively, change the example to be global=false (as is), class-level=true, method-level=false and the explanation should state that all circuit breakers in the app are disabled, only those in the class except the method methodA are enabled.

Fault Tolerance policies can be disabled with configuration at method level, class level or globally for all deployment.
Fault Tolerance policies can be disabled with configuration at method level, class level or globally for all deployment. If multiple configurations are specified, method-level configuration overrides class-level configuration, which then overrides global configuration. e.g.
* `com.acme.test.MyClient/methodA/CircuitBreaker/enabled=true`
* `com.acme.test.MyClient/CircuitBreaker/enabled=false`
Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively, change the example to be global=false (as is), class-level=true, method-level=false and the explanation should state that all circuit breakers in the app are disabled, only those in the class except the method methodA are enabled.

@@ -153,4 +158,4 @@ For instance the following config will disable bulkhead policy globally:

Policy will be disabled everywhere ignoring existing policy annotations on methods and classes.

If the above configurations patterns are used with another value than `false` (i.e. `<classname>/<methodname>/<annotation>/enabled=whatever`) non-portable behaviour results.
If the above configurations patterns are used with another value than `true` or `false` (i.e. `<classname>/<methodname>/<annotation>/enabled=whatever`) non-portable behaviour results.
Copy link
Contributor

Choose a reason for hiding this comment

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

Grammar: "other values than" instead of "another value than"

@Azquelt
Copy link
Member

Azquelt commented May 11, 2018

@OndrejM I've addressed your spec feedback and created TCKs, but I've had to create a new PR because I don't have permission to push directly to branches in this repository.

Would you review my changes in #275? Thanks.

I've also rebased emily's commit onto master to resolve the conflict.

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.

3 participants