Skip to content

fix(ohno): rollback #312#321

Merged
Vaiz merged 1 commit intomainfrom
u/vaiz/2026/03/11/ohno-rollback
Mar 11, 2026
Merged

fix(ohno): rollback #312#321
Vaiz merged 1 commit intomainfrom
u/vaiz/2026/03/11/ohno-rollback

Conversation

@Vaiz
Copy link
Contributor

@Vaiz Vaiz commented Mar 11, 2026

tests show that this workaround doesn't really work for release builds

failures:
    core::tests::from_boxed_string_error

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 28 filtered out; finished in 0.00s

──── STDERR:             ohno core::tests::from_boxed_string_error

thread 'core::tests::from_boxed_string_error' (8632) panicked at crates\ohno\src\core.rs:304:9:
Error("a boxed string error")
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

        FAIL [   0.012s] ohno core::tests::is_boxed_string_error_test
──── STDOUT:             ohno core::tests::is_boxed_string_error_test

running 1 test
test core::tests::is_boxed_string_error_test ... FAILED

tests show that this workaround doesn't really work for release builds
@codecov
Copy link

codecov bot commented Mar 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (8b3a82a) to head (b07bf01).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #321   +/-   ##
=======================================
  Coverage   100.0%   100.0%           
=======================================
  Files         152      152           
  Lines        9351     9337   -14     
=======================================
- Hits         9351     9337   -14     

☔ 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.

@Vaiz Vaiz marked this pull request as ready for review March 11, 2026 13:53
Copilot AI review requested due to automatic review settings March 11, 2026 13:53
@Vaiz Vaiz enabled auto-merge (squash) March 11, 2026 13:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Rolls back the previously introduced ptr_meta/vtable-based workaround in ohno after tests indicated it is unreliable in release builds, returning the crate to a simpler and more stable error-source handling approach.

Changes:

  • Remove the is_boxed_string_error vtable-comparison logic and simplify OhnoCore::from(...) for non-string inputs.
  • Drop the ptr_meta dependency from the workspace and ohno.
  • Update tests and spelling dictionary entries to match the rollback.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/ohno/src/lib.rs Removes the crate-level forbid(unsafe_code) attribute.
crates/ohno/src/core.rs Removes boxed-string-error vtable detection and adjusts From<T> + tests accordingly.
crates/ohno/Cargo.toml Removes ptr_meta dependency from ohno.
Cargo.toml Removes ptr_meta from workspace dependencies.
Cargo.lock Removes ptr_meta package entries/edges after dependency removal.
.spelling Updates custom dictionary entries to reflect removed text.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Vaiz Vaiz merged commit 6ec4849 into main Mar 11, 2026
31 checks passed
@Vaiz Vaiz deleted the u/vaiz/2026/03/11/ohno-rollback branch March 11, 2026 14:28
Copilot AI added a commit that referenced this pull request Mar 11, 2026
…rollback (#321)

Co-authored-by: psandana <1194304+psandana@users.noreply.github.com>
psandana added a commit that referenced this pull request Mar 11, 2026
…rollback (#321)

Co-authored-by: psandana <1194304+psandana@users.noreply.github.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.

4 participants