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

dev: Add Ruff C4 rules #1072

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

dev: Add Ruff C4 rules #1072

wants to merge 1 commit into from

Conversation

ajay-sentry
Copy link
Contributor

Purpose/Motivation

Was looking through the ruff rules list and saw these comprehension style rules which I kinda liked. They all had unsafe-auto fixes available but I was able to go through each one and do a manual replace for all of them

If we like it we can do a similar exercise for Shared / Worker, I think the notation is a lot clearer for me but maybe folks think otherwise and we should scrap this idea.

Rule fixes were:
C401
C403
C405
C416
C417

I also added "C408" to the ignore list because there were like 400 errors for that and they were kinda annoying, particularly for logs. Maybe there were a few that were worth but overall juice wasn't worth the squeeze. Example in the screenshot below

Screenshot 2024-12-26 at 10 08 25 AM

Notes to Reviewer

Anything to note to the team? Any tips on how to review, or where to start?

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

@ajay-sentry ajay-sentry requested review from a team as code owners December 26, 2024 18:08
Copy link

codecov bot commented Dec 26, 2024

Codecov Report

Attention: Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.02%. Comparing base (392c975) to head (992c39c).

Changes have been made to critical files, which contain lines commonly executed in production. Learn more

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
services/bundle_analysis.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1072      +/-   ##
==========================================
- Coverage   96.02%   96.02%   -0.01%     
==========================================
  Files         828      828              
  Lines       19458    19458              
==========================================
- Hits        18685    18684       -1     
- Misses        773      774       +1     
Flag Coverage Δ
unit 92.30% <92.85%> (ø)
unit-latest-uploader 92.30% <92.85%> (ø)

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
Contributor

✅ All tests successful. No failed tests were found.

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

@codecov-notifications
Copy link

Codecov Report

Attention: Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
services/bundle_analysis.py 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

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.

1 participant