KCL: Chamfer and fillet now use keyword arguments #5389
Conversation
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
|
|
adamchalmers
force-pushed
the
achalmers/kw-args-chamfer
branch
from
a860394 to
e23a166
Compare
|
|
adamchalmers
force-pushed
the
achalmers/kw-args-chamfer
branch
from
e23a166 to
53805fe
Compare
|
|
|
|
|
|
adamchalmers
force-pushed
the
achalmers/kw-args-chamfer
branch
from
64b5324 to
adf6ef4
Compare
|
|
|
|
adamchalmers
force-pushed
the
achalmers/kw-args-chamfer
branch
from
4d8b3da to
71bde94
Compare
|
|
adamchalmers
force-pushed
the
achalmers/kw-args-chamfer
branch
from
71bde94 to
002ad0c
Compare
|
|
adamchalmers
force-pushed
the
achalmers/kw-args-chamfer
branch
2 times, most recently
from
5ce73f7 to
1e6c571
Compare
|
|
|
|
adamchalmers
force-pushed
the
achalmers/kw-args-chamfer
branch
from
60874d4 to
cf601c2
Compare
|
|
|
|
|
|
|
|
There was a problem hiding this comment.
👏
I had a quick look over the JS stuff, looks good to me.
I would definitely like @max-mrgrsk to have a chance to take a look too though.
But I think he wrote a lot of tests, so I doubt bugs have been introduced with tests passing.
| @@ -85,7 +85,7 @@ | |||
| "fmt": "prettier --write ./src *.ts *.json *.js ./e2e ./packages", | |||
| "fmt-check": "prettier --check ./src *.ts *.json *.js ./e2e ./packages", | |||
| "fetch:wasm": "./get-latest-wasm-bundle.sh", | |||
| "fetch:samples": "echo \"Fetching latest KCL samples...\" && curl -o public/kcl-samples-manifest-fallback.json https://raw.githubusercontent.com/KittyCAD/kcl-samples/next/manifest.json", | |||
There was a problem hiding this comment.
I'm not super familiar with this, will need to be changed back to next after this breaking change goes live?
| @@ -415,13 +399,13 @@ profile001 = startProfileAt([205.96, 254.59], sketch002) | |||
| cameraPos: { x: 16020, y: -2000, z: 10500 }, | |||
| cameraTarget: { x: -150, y: -4500, z: -80 }, | |||
| beforeChamferSnippet: `angledLine([segAng(rectangleSegmentA001)-90,217.26],%,$seg01) | |||
| chamfer({length=30,tags=[ | |||
| chamfer(extrude001,length=30,tags=[ | |||
There was a problem hiding this comment.
it has been probably answered elsewhere.. We use x=y logic for almost all arguments. Why the reference argument does not follow the same logic? I mean ref=extrude001 instead of just extrude001 ? + It is a little bit confusing that reference has to be the first argument, while the rest can be mixed up.
There was a problem hiding this comment.
The first argument is kinda like a "self" param in Python or other OOP languages. It allows you to make one unlabeled arg which can be omitted if it's % (that way in big |> pipelines you don't need to always use % for the most common cases).
|
|
It was fixed by Kevin in #5446
|
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5389 +/- ##
==========================================
+ Coverage 86.07% 86.14% +0.07%
==========================================
Files 95 95
Lines 35523 35716 +193
==========================================
+ Hits 30576 30768 +192
- Misses 4947 4948 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
|
There was a problem hiding this comment.
Can you delete this file? Also, got.
Previously:
|> fillet({ radius = 5, tags = [seg01] }, %)Now:
|> fillet(radius = 5, tags = [seg01])