-
Notifications
You must be signed in to change notification settings - Fork 17
feat: Remove rounds #1851
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
feat: Remove rounds #1851
Conversation
Remove rounds was in the API server for a while but I am not sure it was working in CoreServices prior to 25.2 so I am using the min backend decorator
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.
Implementation looks perfect. Just left some potential enhancements. I see the tests are failing but once that's in we should be good to merge! Thanks @smereu
update test location
@smereu - tests were failing because the proto API was expecting "selection" and not "faces". I just modified it and committed the change. Tests should pass after this change I guess. BTW - which is the service version in which this API was implemented? Since CoreService is not available earlier than 25R2, it is not a problem. We can probably add the first version in which this API was added |
Okay so it seems that the API was available as of 24.1 (just checked with @b-matteo). So we do not need to add the decorator in that case. I will take over a few changes in this PR to align it to other prepare tools (typehints & docstrings). I want to get it in before we merge blitz into main - and that should happen today. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## blitz #1851 +/- ##
==========================================
- Coverage 90.12% 90.10% -0.02%
==========================================
Files 92 92
Lines 8810 8823 +13
==========================================
+ Hits 7940 7950 +10
- Misses 870 873 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Exposes the ability to remove rounds. The round faces to be removed are input to the method. The functionality was available in previous versions of the API server but CoreServices probably did not implement it until recently. I am using min version 25, 2 decorator but I can change it as needed.
Issue linked
Please mention the issue number or describe the problem this pull request addresses.
Checklist
feat: extrude circle to cylinder
)