rename a shadowed test and re-enable F811 to catch future cases (#8139) #8148
Conversation
…libs#8139) (cherry picked from commit 3c0f1eb)
psf-chronographer
bot
added
the
bot:chronographer:provided
|
@Dreamsorcerer - here's one of the backports |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 3.10 #8148 +/- ##
==========================================
+ Coverage 97.41% 97.44% +0.03%
==========================================
Files 108 108
Lines 32950 32937 -13
Branches 3933 3932 -1
==========================================
Hits 32097 32097
+ Misses 649 637 -12
+ Partials 204 203 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…29d3512419ea65e7cdeb61ba3f3496f00/pr-8139' into patchback/backports/3.10/3c0f1eb29d3512419ea65e7cdeb61ba3f3496f00/pr-8139
|
@Dreamsorcerer I've removed the other shadowed test (made the diff match master) - do you want the contrib file updated for this back port, and if so do I updated both the pr number and the text or just the text? |
|
Ah, I think it's fine. |
| @@ -179,33 +179,7 @@ async def unknown_addrinfo(*args: Any, **kwargs: Any) -> List[Any]: | |||
|
|
|||
|
|
|||
| async def test_close_for_threaded_resolver(loop) -> None: | |||
| resolver = ThreadedResolver(loop=loop) | |||
There was a problem hiding this comment.
@Dreamsorcerer did you miss that this removes an unrelated test in a test module that the original commit didn't even touch?
There was a problem hiding this comment.
@alexmac it seems like the conflict resolution hasn't been completed correctly and this removed a legit test. Why?
There was a problem hiding this comment.
@alexmac was it exactly identical to the other test? It'd be useful to have in a PR description. And perhaps, do such things in separate PRs. Backports must never include changes that aren't directly related to resolving conflicts with the original patch.
There was a problem hiding this comment.
You can see the same test still exists, though the diff makes it a bit awkward to tell. The flake8 rule found this duplicate test that only existed on the 3.x branches. Could have been removed in another PR, but I don't think it makes any real difference.
There was a problem hiding this comment.
Yeah, I managed to find that. It'd be much more obvious (== less mental overhead) if this was explained explicitly.
As for a separate PR — this is the way. Patchback would also work to port from 3.10 to 3.9. The thing is that the git cherry-pick -x is supposed to contain a copy of another change, it even generates a reference (cherry picked from commit xxxxxx). By making arbitrary changes, that makes it hard to track, breaking transparency of the updates to Git. Atomic commits/PRs is a convention that is designed around certain expectations/promises. Breaking them hurts future git paleontology, making the introspection harder. This effect is often not visible/obvious to the commit authors, making it a hard sell, but is quite important for keeping sane history.
(cherry picked from commit 3c0f1eb)
What do these changes do?
Are there changes in behavior for the user?
Is it a substantial burden for the maintainers to support this?
Related issue number
Checklist
CONTRIBUTORS.txtCHANGES/foldername it
<issue_or_pr_num>.<type>.rst(e.g.588.bugfix.rst)if you don't have an issue number, change it to the pull request
number after creating the PR
.bugfix: A bug fix for something the maintainers deemed animproper undesired behavior that got corrected to match
pre-agreed expectations.
.feature: A new behavior, public APIs. That sort of stuff..deprecation: A declaration of future API removals and breakingchanges in behavior.
.breaking: When something public is removed in a breaking way.Could be deprecated in an earlier release.
.doc: Notable updates to the documentation structure or buildprocess.
.packaging: Notes for downstreams about unobvious side effectsand tooling. Changes in the test invocation considerations and
runtime assumptions.
.contrib: Stuff that affects the contributor experience. e.g.Running tests, building the docs, setting up the development
environment.
.misc: Changes that are hard to assign to any of the abovecategories.
Make sure to use full sentences with correct case and punctuation,
for example:
Use the past tense or the present tense a non-imperative mood,
referring to what's changed compared to the last released version
of this project.