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

chore: remove rollout check on test result endpoint #403

Merged
merged 2 commits into from
Feb 22, 2024

Conversation

joseph-sentry
Copy link
Contributor

No description provided.

@joseph-sentry joseph-sentry requested a review from a team as a code owner February 16, 2024 18:20
Copy link

codecov bot commented Feb 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a734bb2) 95.80% compared to head (74d3a95) 95.78%.

❗ Current head 74d3a95 differs from pull request most recent head ab7394a. Consider uploading reports for the commit ab7394a to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##            main    #403     +/-   ##
=======================================
- Coverage   95.80   95.78   -0.02     
=======================================
  Files        757     757             
  Lines      17275   17268      -7     
=======================================
- Hits       16550   16540     -10     
- Misses       725     728      +3     
Flag Coverage Δ
unit 96.08% <ø> (-0.03%) ⬇️
unit-latest-uploader 96.08% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codecov-staging
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@codecov-qa
Copy link

codecov-qa bot commented Feb 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a734bb2) 96.10% compared to head (ab7394a) 96.08%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #403      +/-   ##
==========================================
- Coverage   96.10%   96.08%   -0.03%     
==========================================
  Files         637      637              
  Lines       16693    16686       -7     
==========================================
- Hits        16043    16032      -11     
- Misses        650      654       +4     
Flag Coverage Δ
unit 96.08% <ø> (-0.03%) ⬇️
unit-latest-uploader 96.08% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

codecov-public-qa bot commented Feb 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a734bb2) 96.10% compared to head (ab7394a) 96.08%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #403      +/-   ##
==========================================
- Coverage   96.10%   96.08%   -0.03%     
==========================================
  Files         637      637              
  Lines       16693    16686       -7     
==========================================
- Hits        16043    16032      -11     
- Misses        650      654       +4     
Flag Coverage Δ
unit 96.08% <ø> (-0.03%) ⬇️
unit-latest-uploader 96.08% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
upload/views/test_results.py 98.33% <ø> (-0.16%) ⬇️
utils/rollouts.py 0.00% <ø> (-100.00%) ⬇️

Impacted file tree graph

@joseph-sentry joseph-sentry force-pushed the joseph/rm-test-result-rollout branch 2 times, most recently from b39bed7 to 74d3a95 Compare February 22, 2024 15:37
Copy link

@barnett-yuxiang barnett-yuxiang left a comment

Choose a reason for hiding this comment

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

LGTM

@daniel-codecov
Copy link
Contributor

Just curious, why are we removing this rollout? Do we just want it enabled for everyone now?

@@ -5,18 +5,3 @@

def owner_slug(owner: Owner) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

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

should this get deleted too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's fine to leave this here, it might end up being useful next time someone wants to use a rollout

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think so... that is a helper function, not directly related to this rollout. We can reuse that for future rollouts.
(even if there aren't none right now 😅)

@joseph-sentry
Copy link
Contributor Author

Just curious, why are we removing this rollout? Do we just want it enabled for everyone now?

yeah, we were running into problems where people would want to try it out but they would get blocked on this

@@ -5,18 +5,3 @@

def owner_slug(owner: Owner) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think so... that is a helper function, not directly related to this rollout. We can reuse that for future rollouts.
(even if there aren't none right now 😅)

@joseph-sentry joseph-sentry merged commit 2a0d0e0 into main Feb 22, 2024
16 of 20 checks passed
@joseph-sentry joseph-sentry deleted the joseph/rm-test-result-rollout branch February 22, 2024 16:44
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.

None yet

4 participants