-
Notifications
You must be signed in to change notification settings - Fork 16
refactor: simplify ProfileStatus encoding #1416
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
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1416 +/- ##
=======================================
Coverage 71.66% 71.67%
=======================================
Files 411 411
Lines 66069 66057 -12
=======================================
- Hits 47349 47347 -2
+ Misses 18720 18710 -10
🚀 New features to boost your workflow:
|
BenchmarksComparisonBenchmark execution time: 2025-12-17 21:28:15 Comparing candidate commit 23528d2 in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 56 metrics, 2 unstable metrics. scenario:ip_address/quantize_peer_ip_address_benchmark
CandidateCandidate benchmark detailsGroup 1
Group 2
Group 3
Group 4
Group 5
Group 6
Group 7
Group 8
Group 9
Group 10
Group 11
Group 12
Group 13
Group 14
Group 15
Group 16
Group 17
Group 18
Group 19
BaselineOmitted due to size. |
|
/merge |
|
View all feedbacks in Devflow UI.
This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
The expected merge time in
|
aac786a to
7de41d3
Compare
|
Now, our validation is basically just This lets previously invalid states pass through; I assume that is fine, since we are assuming we probably won't get such invalid states? For example, if Then previously we detected as “allocated but not error” then panic in but now we just treat as OK. Also, if Then previously we panicked and detected that the FFI produced garbage but now our flow of checks are Which is not necessarily correct We are assuming this stuff won't happen or is not an issue? |
What does this PR do?
This simplifies ProfileStatus's internal encoding.
Motivation
I had a feeling it could be done, and when Daniel suggested something in a code review on another PR, it finally clicked.
Additional Notes
This would be a BC break except we haven't made any releases with the old encoding, so it's fine and not a BC break.
How to test the change?
Regular testing is fine.