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.

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

Fix screw holes in parametric enclosure example #1023

Merged
merged 1 commit into from
Mar 15, 2022

Conversation

prplz
Copy link
Contributor

@prplz prplz commented Mar 5, 2022

Updates parametric enclosure example for centerOption changes (#532)

@codecov
Copy link

codecov bot commented Mar 5, 2022

Codecov Report

Merging #1023 (1c07ebf) into master (5147dd8) will not change coverage.
The diff coverage is n/a.

❗ Current head 1c07ebf differs from pull request most recent head 3131a1f. Consider uploading reports for the commit 3131a1f to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1023   +/-   ##
=======================================
  Coverage   96.27%   96.27%           
=======================================
  Files          40       40           
  Lines        9357     9357           
  Branches     1103     1103           
=======================================
  Hits         9008     9008           
  Misses        205      205           
  Partials      144      144           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5147dd8...3131a1f. Read the comment docs.

@prplz
Copy link
Contributor Author

prplz commented Mar 5, 2022

I just realized this could also be fixed by skipping workplane entirely: cutlip.faces(">Z").rect(POSTWIDTH, POSTLENGTH, forConstruction=True).vertices()

@jmwright
Copy link
Member

@prplz I think that skipping the workplane() call is too likely to cause confusion example. Was this the only workplane call that was causing problems, is that why you added centerOption to it but none of the others?

@prplz
Copy link
Contributor Author

prplz commented Mar 15, 2022

This particular example needs it because the lid is moved off center, causing the screw holes to not get added without my change

Copy link
Member

@jmwright jmwright left a comment

Choose a reason for hiding this comment

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

Ok, thanks for the contribution.

@jmwright
Copy link
Member

Simple change, merging with only one review.

@jmwright jmwright merged commit 9e6d323 into CadQuery:master Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants