Skip to content

bump modeling-cmds, nuke slow world #6753

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

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

bump modeling-cmds, nuke slow world #6753

wants to merge 25 commits into from

Conversation

jessfraz
Copy link
Contributor

@jessfraz jessfraz commented May 7, 2025

No description provided.

Copy link

qa-wolf bot commented May 7, 2025

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

Copy link

vercel bot commented May 7, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 13, 2025 2:23am

Copy link

codspeed-hq bot commented May 7, 2025

CodSpeed Instrumentation Performance Report

Merging #6753 will not alter performance

Comparing bump-modeling-cmds (23c96b4) with main (e297f82)

Summary

✅ 54 untouched benchmarks

@@ -292,86 +292,13 @@ pub(crate) async fn do_post_extrude<'a>(
vec![]
};

// Face filtering attempt in order to resolve https://github.com/KittyCAD/modeling-app/issues/5328
Copy link
Contributor

Choose a reason for hiding this comment

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

github wont't let me comment, but a few lines up is

let solid3d_info = args
        .send_modeling_cmd(
            exec_state.next_uuid(),
            ModelingCmd::from(mcmd::Solid3dGetExtrusionFaceInfo {
                edge_id: any_edge_id,
                object_id: sketch.id,
            }),
        )
        .await?;

Should this be removed too @jessfraz

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if you dont need it we dont need it!

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm actually looks like it's used in analyze_faces might just leave it for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we still need that fwiw

Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
jessfraz and others added 3 commits May 9, 2025 13:22
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
jessfraz and others added 4 commits May 10, 2025 10:26
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
jessfraz and others added 2 commits May 12, 2025 17:32
Signed-off-by: Jess Frazelle <github@jessfraz.com>
jessfraz added 2 commits May 12, 2025 18:56
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
Signed-off-by: Jess Frazelle <github@jessfraz.com>
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