Further "unknown marks warning" improvements#5178
Conversation
Codecov Report
@@ Coverage Diff @@
## features #5178 +/- ##
============================================
- Coverage 93.14% 90.67% -2.48%
============================================
Files 115 115
Lines 26139 26124 -15
Branches 2578 2576 -2
============================================
- Hits 24348 23687 -661
- Misses 1471 2106 +635
- Partials 320 331 +11
Continue to review full report at Codecov.
|
| @@ -0,0 +1 @@ | |||
| ``--strict`` is now called ``--strict-markers`` as it is more explicit about what it does. The old name still works for backward compatibility. | |||
There was a problem hiding this comment.
With regard to --strict I would keep it anyway, being able to select also other strict modes later..
There was a problem hiding this comment.
And therefore it could still be used in default config (when the intention here is to enforce strict checkings in general).
There was a problem hiding this comment.
With regard to --strict I would keep it anyway, being able to select also other strict modes later..
It has not been removed, I only added --strict-markers as the now preferred name, --strict is still present and works as before.
There was a problem hiding this comment.
I know.. it just sounded like it would be only kept for backward compatibility, while I think it makes sense to keep it in general.
There was a problem hiding this comment.
And --strict should be used in some test probably still.
There was a problem hiding this comment.
Indeed, I thought the same thing, please see test_strict_prohibits_unregistered_markers
I know.. it just sounded like it would be only kept for backward compatibility, while I think it makes sense to keep it in general.
Not sure if we should ever change the meaning of --strict in the future as well. I mean, suppose we decide to turn "cannot collect classes with __init__" into an error using --strict. If we do that, then test suites which used --strict to turn marks into errors will now break.
In the future we probably should grow another option for that kind of check and leave this option alone?
There was a problem hiding this comment.
@blueyed any thoughts on this? If you feel strongly about changing the meaning of --strict and wording, I don't mind changing it back.
There was a problem hiding this comment.
If we do that, then test suites which used
--strictto turn marks into errors will now break.
I'd argue that this is OK - and easy to fix then.
It is easier to have --strict for CI etc, than having to add multiple --strict-* variants then.
New strictness checks could still get their own option, of course (if it makes sense).
| serial | ||
|
|
||
| **Note**: This option was previously called ``--strict``, which is now an | ||
| alias preserved for backward compatibility. |
There was a problem hiding this comment.
I.e. this could be rephrased.
|
@blueyed gentle ping. 😁 |
|
I'm fine with keeping it like this, but having it clearer would allow problems with deprecations in the future. I.e. if we say now that |
Makes sense, I don't have strong feelings either way to be honest. Can you suggest the changes you would like to see then (either here as "suggestions" or a commit yourself)? |
It doesn't seem to add much value (why would one execute tests based on that marker?), plus using the docstring for that encourages one to write a more descriptive message about the test
b452681 to
0594dba
Compare
Went ahead and implemented the suggested changes to the docs, please let me know what you think. 👍 |
Fix #5023