You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Andrew Gaul <no...@github.com> on 2017/08/01 19:13:01 UTC
[jclouds/jclouds-labs] Parenthesize confusing conditional (#407)
Found via error-prone.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/407
-- Commit Summary --
* Parenthesize confusing conditional
-- File Changes --
M dimensiondata/src/main/java/org/jclouds/dimensiondata/cloudcontrol/handlers/DimensionDataCloudControlErrorHandler.java (9)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/407.patch
https://github.com/jclouds/jclouds-labs/pull/407.diff
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/407
Re: [jclouds/jclouds-labs] Parenthesize confusing conditional (#407)
Posted by Andrew Gaul <no...@github.com>.
Closed #407.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/407#event-1215100331
Re: [jclouds/jclouds-labs] Parenthesize confusing conditional (#407)
Posted by Andrew Gaul <no...@github.com>.
andrewgaul commented on this pull request.
> @@ -49,9 +49,12 @@ public void handleError(HttpCommand command, HttpResponse response) {
case 400:
if (message.contains("RESOURCE_NOT_FOUND") || message.contains("OPERATION_NOT_SUPPORTED")) {
exception = new ResourceNotFoundException(message, exception);
- } else if (message.contains("INVALID_INPUT_DATA") || message.contains("ORGANIZATION_NOT_VERIFIED")
- || message.contains("SYSTEM_ERROR") && !message.contains("RETRYABLE_SYSTEM_ERROR") || message
- .contains("CPU_SPEED_NOT_AVAILABLE") || message.contains("CONFIGURATION_NOT_SUPPORTED")) {
+ } else if ((message.contains("CONFIGURATION_NOT_SUPPORTED") ||
+ message.contains("CPU_SPEED_NOT_AVAILABLE") ||
+ message.contains("INVALID_INPUT_DATA") ||
+ message.contains("ORGANIZATION_NOT_VERIFIED") ||
+ message.contains("SYSTEM_ERROR")) &&
+ !message.contains("RETRYABLE_SYSTEM_ERROR")) {
The intent is not clear to me. @jawnclarke?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/407#discussion_r130761623
Re: [jclouds/jclouds-labs] Parenthesize confusing conditional (#407)
Posted by Andrew Gaul <no...@github.com>.
@jawnclarke Please review.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/407#issuecomment-319468208
Re: [jclouds/jclouds-labs] Parenthesize confusing conditional (#407)
Posted by Andrew Gaul <no...@github.com>.
andrewgaul commented on this pull request.
> @@ -49,9 +49,12 @@ public void handleError(HttpCommand command, HttpResponse response) {
case 400:
if (message.contains("RESOURCE_NOT_FOUND") || message.contains("OPERATION_NOT_SUPPORTED")) {
exception = new ResourceNotFoundException(message, exception);
- } else if (message.contains("INVALID_INPUT_DATA") || message.contains("ORGANIZATION_NOT_VERIFIED")
- || message.contains("SYSTEM_ERROR") && !message.contains("RETRYABLE_SYSTEM_ERROR") || message
- .contains("CPU_SPEED_NOT_AVAILABLE") || message.contains("CONFIGURATION_NOT_SUPPORTED")) {
+ } else if ((message.contains("CONFIGURATION_NOT_SUPPORTED") ||
+ message.contains("CPU_SPEED_NOT_AVAILABLE") ||
+ message.contains("INVALID_INPUT_DATA") ||
+ message.contains("ORGANIZATION_NOT_VERIFIED") ||
+ message.contains("SYSTEM_ERROR")) &&
+ !message.contains("RETRYABLE_SYSTEM_ERROR")) {
Reading this more closely, I agree with your interpretation.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/407#discussion_r134369208
Re: [jclouds/jclouds-labs] Parenthesize confusing conditional (#407)
Posted by Andrew Phillips <no...@github.com>.
demobox requested changes on this pull request.
> @@ -49,9 +49,12 @@ public void handleError(HttpCommand command, HttpResponse response) {
case 400:
if (message.contains("RESOURCE_NOT_FOUND") || message.contains("OPERATION_NOT_SUPPORTED")) {
exception = new ResourceNotFoundException(message, exception);
- } else if (message.contains("INVALID_INPUT_DATA") || message.contains("ORGANIZATION_NOT_VERIFIED")
- || message.contains("SYSTEM_ERROR") && !message.contains("RETRYABLE_SYSTEM_ERROR") || message
- .contains("CPU_SPEED_NOT_AVAILABLE") || message.contains("CONFIGURATION_NOT_SUPPORTED")) {
+ } else if ((message.contains("CONFIGURATION_NOT_SUPPORTED") ||
+ message.contains("CPU_SPEED_NOT_AVAILABLE") ||
+ message.contains("INVALID_INPUT_DATA") ||
+ message.contains("ORGANIZATION_NOT_VERIFIED") ||
+ message.contains("SYSTEM_ERROR")) &&
+ !message.contains("RETRYABLE_SYSTEM_ERROR")) {
@andrewgaul I'm not sure this is the same? As I read the original, it's
`... || ... || (message.contains("SYSTEM_ERROR") && !message.contains("RETRYABLE_SYSTEM_ERROR")) || ... || ...`
which seems to be different from the new version?
E.g. if `message.contains("INVALID_INPUT_DATA") == true` and `message.contains("RETRYABLE_SYSTEM_ERROR") == true`, the old version would evaluate to true but the new version would evaluate to false?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/407#pullrequestreview-53669152