{{ message }}
Remove some unused modules #8630
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8630 +/- ##
==========================================
- Coverage 59.60% 59.60% -0.01%
==========================================
Files 391 391
Lines 39286 39280 -6
==========================================
- Hits 23418 23412 -6
Misses 15868 15868 ☔ View full report in Codecov by Sentry. |
|
started regression test run here |
emolter
reviewed
Jul 16, 2024
There was a problem hiding this comment.
These changes look good to me, pending regtests; indeed, none of these modules are used. It seems to beg the question of why our ruff check didn't catch these earlier. @zacharyburnett maybe that's a question for you?
it looks like Lines 276 to 285 in 8af184c I copied this config from the previous |
|
looks like these exclusions were added in 9be1e27 |
|
I ran rules |
There was a problem hiding this comment.
These changes look good to me as well, and it does also sound like a good idea to remove those exclusions from the style checks. Thanks for spotting that, @emolter, and taking care of it @zacharyburnett.
I think it was even earlier - that commit just split the exclusions across multiple lines. Thanks for starting this discussion @AlexKurek - it sounds like we should go ahead and merge this one assuming the regression tests don't turn up any surprises, and then look at #8651 separately. Does that sound ok to you @zacharyburnett ? |
sounds good to me! |
made a separate PR to remove those exclusions #8652 |
emolter
approved these changes
Jul 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves JP-nnnn
Closes #
This PR addresses ...
Checklist for PR authors (skip items if you don't have permissions or they are not applicable)
CHANGES.rstwithin the relevant release sectionHow to run regression tests on a PR