Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
until extrude/cutblind #875
until extrude/cutblind #875
Changes from 13 commits
529dccb
54e5939
01e2266
752a485
fef12cf
1897790
e5a5fe2
2553740
761a3a5
236be05
ec64bcf
7e53027
4e324e4
6cd4d2f
2d142d0
5878b17
b05242f
84aaff5
40ac712
54a48b1
c369e5d
0fded0d
e2141c4
bdec80b
7b371d0
a91f623
20691bc
da62886
8dab694
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I may be misunderstanding your intention here, but I think you meant:
I would suggest a test is added for this as well.
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.
I think you misunderstood, this part of the code does :
upToface
is of typeint
orFace
, if it's typeint
it checks that if an extrusion is required and the extrusion until the next face is required, if the wire is insinde the solid then we want to extrude until the next face Outside the solid. Hence theupToFace = 1
upToFace = 0
int
then it's of type `Face` and we pass the face to extrude to directlyThere is indeed no check if
upToFace
is actually of typeUnion[int, Face]
. Would you want to add a type check at the beginning and raise an error if that is not the case ? (And add a test associated with it)I don't know if it's relevant since the allowed type are given in the method signature and it's a private method. A type check is already performed on the public method
extrude
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.
I didn't mean types.
Then you probably want to set
upToFace = 1
before you dolimitFace = facesList[upToFace]
?