Small things #37
Small things #37
Conversation
There was a problem hiding this comment.
Mostly good, some nit, and I would rather we not remove the RelativePathError without good cause. Sure, reducing the number of errors is nice, but OpcUaError might quickly grow very large if we include all possible error modes from all possible functions in it. It's probably better for the future to have individual error enums where it makes sense, then just include those in OpcUaError. Doing so is easy with thiserror anyway.
| @@ -340,21 +382,6 @@ impl RelativePathElement { | |||
| } | |||
| } | |||
|
|
|||
| #[derive(Debug, Clone)] | |||
| /// Error returned from parsing a relative path. | |||
| pub enum RelativePathError { | |||
There was a problem hiding this comment.
Could you, instead of removing this error, just imlement From<RelativePathError> for OpcUaError? I don't see a good reason to flatten the error now.
Do feel free to derive Error on RelativePathError, just using the docstrings as error messages.
There was a problem hiding this comment.
yes might be. I was unsure what the correct approach is here. I just proposed something.
And yes maybe From<RelativePathError> for OpcUaError is a good solution.
I will change that
oroulet
force-pushed
the
smallthings
branch
2 times, most recently
from
d304b29 to
56786bc
Compare
No description provided.