ref(logging): Clarify separate warnings case is for Python <3.11 #4296
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #4296 +/- ##
==========================================
- Coverage 79.54% 79.52% -0.03%
==========================================
Files 142 142
Lines 15907 15907
Branches 2722 2721 -1
==========================================
- Hits 12654 12650 -4
- Misses 2389 2391 +2
- Partials 864 866 +2
|
| } | ||
|
|
||
| if ( | ||
| sys.version_info < (3, 11) |
There was a problem hiding this comment.
Note: this additional version check does not change actual behavior. In Python 3.11+, record.msg contains the actual warning message, so the later on record.msg == "%s" check would fail. Adding this version check just makes it more obvious that this code path only exists for older Python versions
The way the code was written before this change made it look like log records from the `warnings` module were always being handled by a separate code path. In fact, this separate path is only used for Python 3.10 and below. This change makes it clear that the branch is version specific. That way, when we eventually stop supporting 3.10, it is clear that we can delete this separate block. Depends on: - #4292 - #4291
0501ee7 to
caa7c3a
Compare
Generally agree with your point here, but after removing the Besides removing the
|
The way the code was written before this change made it look like log records from the
warningsmodule were always being handled by a separate code path. In fact, this separate path is only used for Python 3.10 and below.This change makes it clear that the branch is version specific. That way, when we eventually stop supporting 3.10, it is clear that we can delete this separate block.
Depends on: