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

ref(mqb): Remove the flag for defaulting tags #71697

Closed
wants to merge 7 commits into from

Conversation

wmak
Copy link
Member

@wmak wmak commented May 29, 2024

  • This flag has been GA'ed for a while now so lets just default it now since we should always do this

- This flag has been GA'ed for a while now so lets just default it now
  since we should always do this
@wmak wmak requested review from a team as code owners May 29, 2024 18:45
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label May 29, 2024
@wmak wmak changed the title ref(trace): Remove the flag for defaulting tags ref(mqb): Remove the flag for defaulting tags May 29, 2024
Copy link

codecov bot commented May 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.70%. Comparing base (5aeaf13) to head (a979222).
Report is 1647 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #71697      +/-   ##
==========================================
- Coverage   78.01%   77.70%   -0.32%     
==========================================
  Files        6632     6631       -1     
  Lines      296035   295786     -249     
  Branches    50986    50934      -52     
==========================================
- Hits       230957   229834    -1123     
- Misses      58794    59618     +824     
- Partials     6284     6334      +50     
Files Coverage Δ
src/sentry/features/temporary.py 100.00% <ø> (ø)
src/sentry/search/events/builder/metrics.py 45.86% <100.00%> (-42.66%) ⬇️
src/sentry/search/events/builder/spans_metrics.py 100.00% <100.00%> (ø)

... and 176 files with indirect coverage changes

Copy link
Member

@mjq mjq left a comment

Choose a reason for hiding this comment

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

👍

@getsantry
Copy link
Contributor

getsantry bot commented Jun 21, 2024

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you add the label WIP, I will leave it alone unless WIP is removed ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added the Stale label Jun 21, 2024
Comment on lines +673 to +675
(len(self.valid_tags) > 0 and value in self.valid_tags)
or len(self.valid_tags) == 0
or self.use_on_demand
Copy link
Member

Choose a reason for hiding this comment

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

Nit: check the override cases first for better legibility and remove the redundant len > 0 check

Suggested change
(len(self.valid_tags) > 0 and value in self.valid_tags)
or len(self.valid_tags) == 0
or self.use_on_demand
len(self.valid_tags) == 0
or self.use_on_demand
or value in self.valid_tags

@getsantry
Copy link
Contributor

getsantry bot commented Aug 5, 2024

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you add the label WIP, I will leave it alone unless WIP is removed ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added the Stale label Aug 5, 2024
@getsantry getsantry bot closed this Aug 13, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants