-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
skipping: factor out _get_pos, pass only config to _get_report_str #5002
Conversation
Codecov Report
@@ Coverage Diff @@
## features #5002 +/- ##
============================================
+ Coverage 95.97% 95.97% +<.01%
============================================
Files 113 113
Lines 25359 25365 +6
Branches 2501 2501
============================================
+ Hits 24339 24345 +6
Misses 714 714
Partials 306 306
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## features #5002 +/- ##
============================================
+ Coverage 95.97% 95.97% +<.01%
============================================
Files 113 113
Lines 25359 25365 +6
Branches 2501 2501
============================================
+ Hits 24339 24345 +6
Misses 714 714
Partials 306 306
Continue to review full report at Codecov.
|
I'm curious, in order to jump to the code using your editor? |
Yeah, It was mainly inspired when looking at Teemu/pytest-sugar#173. I am using a Vim pluging that handles pytest's nodeids also (https://github.com/wsdjeg/vim-fetch), but there then the lineno could be useful also. I was thinking about changing the default to "nodeid, line X" at least - but allow for configuring this through an ini option, so that e.g. |
I'm 👍 to change the default, but I don't think we need an option if that default is good enough for:
What do you think? |
I agree with not having unnecessary options, especially since this would involve a mini template engine in this case ("lineno" might be not available, and then you do not want "fname:" I guess). I think we should retain |
Oh good point, it will probably be too long indeed... |
So not a good default then? I've also thought about extending the nodeid, i.e. |
Merging as-is already, happy to follow up the discussion here (or in a new issue, whatever you prefer). |
This is for "short summary".
I've started looking into making the format there configurable, to support e.g. "fname:lineno" instead of nodeids.