Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Implement support for pointxyz and range leaves #81

Merged
merged 1 commit into from
Apr 5, 2023
Merged

Implement support for pointxyz and range leaves #81

merged 1 commit into from
Apr 5, 2023

Conversation

knzivid
Copy link
Contributor

@knzivid knzivid commented Mar 31, 2023 •
edited
Loading

No description provided.

@knzivid
Copy link
Contributor Author

knzivid commented Apr 3, 2023

RFNR

apartridge
apartridge reviewed Apr 3, 2023
View reviewed changes
Copy link
Collaborator

@apartridge apartridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR looks good! I will just take a closer look at it tomorrow and test it a bit.

@apartridge
Copy link
Collaborator

The code is looking good, I tested it and it works as expected. I just added a few comments on the test. If you can fix those I will approve.

I think we can do the bump to 2.9 in a separate PR, as we also want to update README etc.

@knzivid
Copy link
Contributor Author

knzivid commented Apr 4, 2023

Unless you have any other feedback, can you approve the PR?

@knzivid knzivid marked this pull request as ready for review April 4, 2023 10:49
@apartridge
Copy link
Collaborator

#81 (comment)

@knzivid
Copy link
Contributor Author

knzivid commented Apr 4, 2023

Remember to unresolve the thread if you comment on them :)

@knzivid
Copy link
Contributor Author

knzivid commented Apr 4, 2023

RFNR

@knzivid knzivid requested a review from apartridge April 4, 2023 12:41
@knzivid
Copy link
Contributor Author

knzivid commented Apr 4, 2023

PS: The CI looks a bit dead, probably because you are making this PR from your own fork?

Can you allow CI for me as an external contributor then?

apartridge
apartridge previously approved these changes Apr 4, 2023
View reviewed changes
apartridge
apartridge previously approved these changes Apr 4, 2023
View reviewed changes
@knzivid knzivid force-pushed the roi-ros branch 4 times, most recently from a848cf5 to b6e9a15 Compare April 4, 2023 13:29
apartridge
apartridge previously approved these changes Apr 4, 2023
View reviewed changes
@knzivid knzivid merged commit bcd7770 into zivid:master Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apartridge apartridge apartridge approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@knzivid @apartridge