Skip to content
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

Remove combining_cov #226

Closed
wants to merge 2 commits into from
Closed

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Sep 16, 2018

It turns out that this is only required as a workaround to
overwriting/setting self.cov.config.paths['source'] (in a4fc404), which
overrides the (relative) value from the config.

xdist nodes are still appending their source mapping.

@blueyed blueyed force-pushed the fix-combine branch 2 times, most recently from 244d77e to 3d8cf61 Compare September 16, 2018 14:43
It turns out that this is only required as a workaround to
overwriting/setting `self.cov.config.paths['source']` (in a4fc404), which
overrides the (relative) value from the config.

xdist nodes are still appending their source mapping.
@blueyed blueyed changed the title WIP: fix combining Remove combining_cov Sep 21, 2018
@blueyed
Copy link
Contributor Author

blueyed commented Sep 21, 2018

Closing, it is required for a clean data_file, and there appears to be not better API it seems.

@ionelmc
Copy link
Member

ionelmc commented Nov 16, 2018

@blueyed Huh. So the fix was this simple (just remove the source patching)?

Would this fix all these complaints people have been making about the new combining behavior in 2.6.0?

@blueyed
Copy link
Contributor Author

blueyed commented Nov 16, 2018

@ionelmc
What are you referring to exactly (re "just remove the source patching")?

(AFAIR one main issue I had was that running "coverage combine" manually on CI would not work anymore, since pytest-cov did it already)

@blueyed
Copy link
Contributor Author

blueyed commented Nov 16, 2018

If you are referring to a4fc404#diff-327bb273d551686616daadf60180a4d9R169 being wrong maybe, then that could be the case - I'm not sure about the details currently anymore.

@ionelmc
Copy link
Member

ionelmc commented Nov 16, 2018

Ah ... nevermind. I thought this was merged (now I see it's just closed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants