-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(preprod): skip comparisons for different analysis versions #103195
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
| diff_items: list[DiffItem] | ||
| size_metric_diff_item: SizeMetricDiffItem | ||
| skipped_diff_item_comparison: bool | ||
| head_analysis_version: str | None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't sure if the analysis versions should go in SizeMetricDiffItem or not.
| class ComparisonResults(BaseModel): | ||
| diff_items: list[DiffItem] | ||
| size_metric_diff_item: SizeMetricDiffItem | ||
| skipped_diff_item_comparison: bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can look for this field in the frontend to show a nice UI around "You should rebase..."
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #103195 +/- ##
===========================================
- Coverage 80.67% 80.67% -0.01%
===========================================
Files 9241 9241
Lines 393627 393670 +43
Branches 25053 25053
===========================================
+ Hits 317543 317576 +33
- Misses 75622 75632 +10
Partials 462 462 |
| head_size = head_element.size | ||
| if head_size == 0: | ||
| continue | ||
| if not skip_diff_item_comparison: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should log the fact we're skipping if so
Resolves EME-629
For now this just handles skipping the treemap diff item comparisons, although we also spoke about perhaps skipping the diff altogether if the major versions differ.