-
-
Notifications
You must be signed in to change notification settings - Fork 92
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
Add an "Align.NONE" enum member. #734
base: dev
Are you sure you want to change the base?
Conversation
This option means "leave me alone". It is necessary for placing text on its baseline.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #734 +/- ##
==========================================
- Coverage 96.00% 95.88% -0.12%
==========================================
Files 25 25
Lines 8351 8362 +11
==========================================
+ Hits 8017 8018 +1
- Misses 334 344 +10 ☔ View full report in Codecov by Sentry. |
A few lines of unittests are required - patch coverage should be 100%. |
Oh, how about changing |
I can do that. However … having the same set of tests in no less than five different locations is a red flag. Time to refactor that instead of adding more repetitive code (and tests for it). Ideas for a good function name? I tend towards |
What are you proposing to change? |
This code
appears five times in build123d's codebase. That's four times too many. Also I'd like to calculate the coordinates once each instead of up to three times, plus I don't like to rely on typechecking to prevent results like the one you get if you |
Sure, that code could be improved. There is an related issue #457 - are you planning on fixing this too? |
Thanks for the pointer. Yes. |
This option means "leave me alone".
It is necessary for placing text on its baseline.