Github user spmallette commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/149#issuecomment-158979326
it's nice that you have "root cause" as optional to avoid breaking change.
in that way, it is optional for providers to pass a cause and if they just want
a vanilla exception they can use it that way. I guess that opens up the
opportunity for there to be some inconsistency in the exceptions as one
provider might choose to include cause and the other might not have a cause in
the first place to throw....that might not matter i suppose.
I also wonder if all exceptions should have this option. It's a bigger
change, but perhaps all exception methods should be deprecated and instead
return some sort of `ExceptionBuilder<T extends Throwable>`. That way, the
provider could do something like:
```text
throw
Property.Exceptions.buildDataTypeOfPropertyValueNotSupported(value).cause(root).create();
```
or
```text
throw
Property.Exceptions.buildDataTypeOfPropertyValueNotSupported(value).create();
```
as the need was required. Then this would be consistently applied. Of
course, perhaps this specific issue is isolated as a problem and we aren't
hiding "native" causes and messages elsewhere. If that's the case, then this
pull request is the "simple" thing to do.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
