-
-
Notifications
You must be signed in to change notification settings - Fork 578
Add tests for Spline tool to support continuing a previously drawn spline #2591
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
Conversation
If you were planning to continue adding more tests from #2483, please don't start any new ones until #2368 is merged. Please also avoid opening too many PRs for small sub-tasks in one of those issues that have multiple bullet points, just add them all to the same PR if you can and they're related. It adds a significant amount of overhead dealing with multiple PRs. |
Noted. Shall I close this one or keep it as draft? |
Please finish this one, but after it's ready to merge, don't work on new Spline tool things until after that other issue lands. |
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.
Looks good so far.
@0HyperCube I've refactored the redundant code across the three open PRs. Would it make sense to include this in |
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.
Looks good; thanks for simplifying the test code.
I think the assert_point_positions
is fine in the spline tool as it is useful for that tool only.
Closes part of #2483