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.

Sign up for GitHub

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

Jump to bottom

[ALRT-39] fix comparison query timestamp column #70820

Merged
merged 6 commits into from
May 21, 2024
Merged

[ALRT-39] fix comparison query timestamp column #70820

merged 6 commits into from
May 21, 2024
+105 −0

Conversation

nhsiehgit
Copy link
Member

@nhsiehgit nhsiehgit commented May 13, 2024 •
edited by ceorourke

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label May 13, 2024
@nhsiehgit nhsiehgit force-pushed the nhsieh/fix_comparison_query branch from 1963818 to ce9078b Compare May 13, 2024 22:06
@nhsiehgit nhsiehgit marked this pull request as ready for review May 13, 2024 22:06
@nhsiehgit nhsiehgit requested a review from a team as a code owner May 13, 2024 22:06
@ceorourke
Copy link
Member

Nice, would it be too much to ask to add a test for get_comparison_aggregation_value?

@codecov Codecov
Copy link

codecov bot commented May 13, 2024 •
edited

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.86%. Comparing base (2b255d0) to head (80765d4).
Report is 73 commits behind head on master.

Additional details and impacted files 
@@             Coverage Diff             @@
##           master   #70820       +/-   ##
===========================================
+ Coverage   61.80%   77.86%   +16.05%     
===========================================
  Files        3662     6528     +2866     
  Lines      112673   290944   +178271     
  Branches    18342    50342    +32000     
===========================================
+ Hits        69643   226557   +156914     
- Misses      42651    58132    +15481     
- Partials      379     6255     +5876
Files Coverage Δ
src/sentry/search/events/builder/errors.py 97.18% <100.00%> (ø)

... and 2865 files with indirect coverage changes

@volokluev
Copy link
Member

This needs a test. If you can't reproduce the issue in a test then how will you know that it was fixed?

@nhsiehgit
Copy link
Member Author

working on the tests now

@nhsiehgit nhsiehgit changed the title fix comparison query timestamp column [ALRT-39] fix comparison query timestamp column May 16, 2024
@ceorourke ceorourke force-pushed the nhsieh/fix_comparison_query branch from e9a5f30 to 5a0e0e0 Compare May 21, 2024 01:23
@ceorourke ceorourke requested review from a team as code owners May 21, 2024 01:23
nhsiehgit
nhsiehgit commented May 21, 2024
View reviewed changes
Copy link
Member Author

@nhsiehgit nhsiehgit left a comment

Choose a reason for hiding this comment

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

+1

@nhsiehgit nhsiehgit merged commit 1d6ac0c into master May 21, 2024
49 checks passed
@nhsiehgit nhsiehgit deleted the nhsieh/fix_comparison_query branch May 21, 2024 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kylemumma kylemumma kylemumma left review comments

@ceorourke ceorourke ceorourke left review comments

@wedamija wedamija wedamija approved these changes

@volokluev volokluev Awaiting requested review from volokluev

Assignees
No one assigned
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@nhsiehgit @ceorourke @volokluev @wedamija @kylemumma