Skip to content

Enable coveragerc #21

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

Closed
wants to merge 2 commits into from
Closed

Conversation

bukzor
Copy link
Contributor

@bukzor bukzor commented Oct 22, 2014

The essential idea is that there should be a way to enable pytest-cov without overriding the source setting in .coveragerc.

The new test requires the pull request to cov-core here: schlamar/cov-core#5

This is my second implementation. You can compare to the first here:
Find the diff here: bukzor@ebfcf41

@bukzor bukzor force-pushed the enable-coveragerc branch 3 times, most recently from b02431e to 7a42117 Compare October 23, 2014 01:04
@bukzor bukzor force-pushed the enable-coveragerc branch from 7a42117 to 9fdffc7 Compare October 23, 2014 21:16
@schlamar
Copy link
Contributor

Hey, thanks for your contributions. I'm planning a bigger rewrite right now (merging cov-core into the plugin, better re-use of coverage's features, ...). A lot of your issues should be solved by then, so please stay tight, I guess this takes a while. But I'm coming back to you :)

@bukzor
Copy link
Contributor Author

bukzor commented Oct 27, 2014

This PR also contains tests that your rewrite should pass.
Please don't let the future be an enemy of the present.

@bukzor
Copy link
Contributor Author

bukzor commented Nov 17, 2014

Please to pull.

I'm currently forced to use a fork with a git: url, and I'd much rather not.

@schlamar
Copy link
Contributor

I'm not quite happy about introducing a new command line argument which is directly connected to a second one. I have thought about passing a special keyword to --cov but I'm not sure if that's actually better.

What do you think about implicitly enabling coverage if a coveragerc file is present and introducing something like --no-cov to explicitely disable it?

@bukzor
Copy link
Contributor Author

bukzor commented Nov 18, 2014

I'm pretty sure you'll get bugs down that path, where people have coverage enabled where they didn't expect it.

My proposal just makes the current approach more explicit. The current version enables pytest-cov whenever a source is configured. This makes the --source option do double duty: configuring coverage's --source and enabling the coverage system. This leaves no way to enable the system without over-ridding any source configured in coveragerc.

After my change, --source still does the same double duty, just more explicitly, and there's a way to enable the system without over-ridding [run]source.

The only way to make the change smaller would be to provide some magic value to --cov-source, maybe --cov-source=None, but this seems both non-intuitive and magical.

@bukzor
Copy link
Contributor Author

bukzor commented Nov 18, 2014

We can resolve these issues much more efficiently interactively.
If you're willing, find me on freenode at buck1.

@schlamar
Copy link
Contributor

Just released cov-core 1.15.0. Please rebase and update this PR.

@schlamar
Copy link
Contributor

(Yes, that means I favor this version ;) )

@schlamar
Copy link
Contributor

Please don't let the future be an enemy of the present.

Feel free to join discussion, add ideas, etc. at #28 :)

@schlamar
Copy link
Contributor

As third option (instead of this or #26) we could introduce a non-backward compatible parameter to 2.0.

@schlamar
Copy link
Contributor

Here is my proposal (should go into 2.0):

  • --cov is a flag to enable coverage / activate the plugin
  • --cov-source is the default option to specify sources
  • if arguments are passed to --cov, they are still accepted to specify the source, but a deprecation warning will be printed

@bukzor
Copy link
Contributor Author

bukzor commented Nov 23, 2014

That sounds like an enhancement to this patch, i.e. can be done in
subsequent pull requests.

--phone is hard.
On Nov 23, 2014 3:56 AM, "Marc Schlaich" [email protected] wrote:

Here is my proposal (should go into 2.0):

  • --cov is a flag to enable coverage / activate the plugin
  • --cov-source is the default option to specify sources
  • if arguments are passed to --cov, they are still accepted to specify
    the source, but a deprecation warning will be printed


Reply to this email directly or view it on GitHub
#21 (comment).

@schlamar schlamar mentioned this pull request Nov 24, 2014
12 tasks
@schlamar
Copy link
Contributor

#35

@schlamar schlamar closed this Nov 24, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants