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

Feature: Release named views to all users #5814

Merged
merged 42 commits into from
Mar 26, 2025

Conversation

nadr0
Copy link
Collaborator

@nadr0 nadr0 commented Mar 14, 2025

Issue

Engine + Modeling API was not updated till as of late. We now have the modeling events and API!

Implementation

  • added E2E tests for named views
  • added snapshot logic to the E2E test for the toml string for easier matching of data
  • removed DEV flag for named views to be released to everyone
  • bumped "@kittycad/lib": "2.0.21", to support the new APIs.
  • bumping the library made me update the export model type to 3d
  • removed all skip_serializing_if = "is_default")] on NamedView because I need them explicitly written because when loading them to the engine I need all of the values defined. This cannot be an implicit struct.
  • fought some typescript to convert CameraViewState to NamedView and vice versa since the API and NamedView struct ended up with different ts-rs generated types :(
  • error handling in the load and create named view workflows because of the type checking

Copy link

qa-wolf bot commented Mar 14, 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 Mar 14, 2025

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

Name Status Preview Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview Mar 26, 2025 3:45pm

@nadr0 nadr0 changed the title Nadro/adhoc/named views release Feature: Release named views to all users Mar 14, 2025
@nadr0 nadr0 requested a review from a team March 14, 2025 21:07
Copy link

codecov bot commented Mar 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.83%. Comparing base (0c4826c) to head (2b2ba8c).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5814      +/-   ##
==========================================
- Coverage   85.83%   85.83%   -0.01%     
==========================================
  Files         113      113              
  Lines       44484    44484              
==========================================
- Hits        38185    38182       -3     
- Misses       6299     6302       +3     
Flag Coverage Δ
rust 85.83% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jessfraz
Copy link
Contributor

thank you for all the test!

Copy link

codspeed-hq bot commented Mar 14, 2025

CodSpeed Walltime Performance Report

Merging #5814 will degrade performances by 11.59%

Comparing nadro/adhoc/named-views-release (7abb23c) with main (3b2abe5)

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

Summary

❌ 1 regressions
✅ 87 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
execute_walkie-talkie 4.5 s 5.1 s -11.59%

@lf94
Copy link
Contributor

lf94 commented Mar 17, 2025

@nadr0 will review today.

@pierremtb
Copy link
Collaborator

Fixing up snapshot conflicts so we can merge this beauty

@pierremtb pierremtb merged commit 1753047 into main Mar 26, 2025
56 of 58 checks passed
@pierremtb pierremtb deleted the nadro/adhoc/named-views-release branch March 26, 2025 16:12
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.

5 participants