-
Notifications
You must be signed in to change notification settings - Fork 403
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
iox-#1969 align iox expected to std expected and add convenience free functions #1981
iox-#1969 align iox expected to std expected and add convenience free functions #1981
Conversation
1720ed6
to
af7f994
Compare
Codecov Report
@@ Coverage Diff @@
## master #1981 +/- ##
==========================================
- Coverage 74.23% 74.13% -0.11%
==========================================
Files 413 405 -8
Lines 16067 15942 -125
Branches 2250 2242 -8
==========================================
- Hits 11928 11818 -110
+ Misses 3425 3413 -12
+ Partials 714 711 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM content-wise, thanks! Sorry for being a smug here, but would you mind updating the year in the copyright headers? 😅
@@ -51,7 +51,7 @@ class FunctionalInterface_test : public testing::Test | |||
}; | |||
|
|||
/// @brief This types is used for testing the functional interface in the case | |||
/// of a value and a get_error method | |||
/// of a 'value' and a 'error' method |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
using DummyValueType = bool; | ||
static constexpr DummyValueType DUMMY_VALUE{true}; | ||
|
||
iox::variant<DummyValueType, ErrorType> data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that should be enough, right?
iox::variant<DummyValueType, ErrorType> data; | |
variant<DummyValueType, ErrorType> data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The additional namespace does not hurt :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it's not needed :)
/// @brief helper struct to create an expected which is signalling success more easily | ||
/// @param T type which the success helper class should contain | ||
template <typename T = void> | ||
using success = detail::ok<T>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be deprecated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved under the condition that the copyright header is updated in a follow-up.
Sorry, all relevant copyright header are updated. A mass search and replace does not warrant an update of the copyright header. |
Pre-Review Checklist for the PR Author
iox-123-this-is-a-branch
)iox-#123 commit text
)task-list-completed
)iceoryx_hoofs
are added to./clang-tidy-diff-scans.txt
Notes for Reviewer
This PR aligns the
iox::expected
better to thestd::expected
expected<E>
in favor ofexpected<void, E>
get_error
witherror
has_value
ok
anderror
free functionsunexpect_t
helper struct for direct initializationChecklist for the PR Reviewer
Copyright owner are updated in the changed filesiceoryx_hoofs
have been added to./clang-tidy-diff-scans.txt
Post-review Checklist for the PR Author
References
expected<void, Error>
is unusable due tofinal
#1976