-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Extension of Nd range #1449
Extension of Nd range #1449
Conversation
I have implemented the changes discussed in the spec as well as the changes concerning |
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.
Other than that LGTM
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!
Signed-off-by: Tobias Weinzierl <[email protected]>
Signed-off-by: Tobias Weinzierl <[email protected]>
5b3d1a5
to
99fc103
Compare
Co-authored-by: Konstantin Boyarinov <[email protected]>
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!
Signed-off-by: pavelkumbrasev <[email protected]>
fa0db8e
Description
We found that it is very useful for our code if we can construct the range from plain integers. For this, the underlying datatype storing the ranges has to be changed to vector. This should have no negative impact on the performance. However, it makes using the class easier.
The class could do with more assertions and more docu.
Type of change
Choose one or multiple, leave empty if none of the other choices apply
Add a respective label(s) to PR if you have permissions
Tests
Documentation
Breaks backward compatibility
Notify the following users
List users with
@
to send notificationsOther information