Skip to content

Fix to add NodePaths to SketchOnFace and SketchOnPlane artifacts #6885

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

Closed
wants to merge 1 commit into from

Conversation

jtran
Copy link
Contributor

@jtran jtran commented May 12, 2025

Stacked on #6884 so that we can see the fix is working.

The "Missing NodePath"s that are left are all SourceRanges pointing into another module.

Copy link

vercel bot commented May 12, 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 14, 2025 6:00am

Copy link

codspeed-hq bot commented May 12, 2025

CodSpeed Instrumentation Performance Report

Merging #6885 will not alter performance

Comparing jtran/fix-sketch-on-face-node-path (6c9f39e) with main (1e487ef)

Summary

✅ 70 untouched benchmarks

@jtran jtran force-pushed the jtran/fix-sketch-on-face-node-path branch from 439d22d to 352a2fa Compare May 13, 2025 19:44
@jtran jtran changed the base branch from main to jtran/node-path-output May 13, 2025 19:44
@jtran jtran requested a review from Irev-Dev May 13, 2025 19:46
Copy link
Contributor

@Irev-Dev Irev-Dev left a comment

Choose a reason for hiding this comment

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

Looks good,

And yeah don't think we need to cover the import cases right now.

@jtran jtran force-pushed the jtran/node-path-output branch from 36398db to c50391c Compare May 13, 2025 21:57
Base automatically changed from jtran/node-path-output to main May 14, 2025 05:31
@jtran
Copy link
Contributor Author

jtran commented May 14, 2025

WTF. After getting #6884 merged and rebasing, the output update that shows this works is gone. This is what it used to look like: 352a2fa.

@jtran
Copy link
Contributor Author

jtran commented May 15, 2025

WTF. After getting #6884 merged and rebasing, the output update that shows this works is gone. This is what it used to look like: 352a2fa.

Resolved here: #6981.

@jtran
Copy link
Contributor Author

jtran commented May 15, 2025

I have an improved way to do this, and I'm going to roll it into #6978.

@jtran jtran closed this May 15, 2025
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