Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Adds an option to specify the location of the coverage config file. #896

Conversation

brian-reinhart
Copy link
Contributor

Adds an option to specify the location of the coverage config file.

@jszakmeister
Copy link
Contributor

What's the motivation here? Why do you not have the coverage config file in the same directory that you're running nose? Also, not much gets merged these days without tests.

@brian-reinhart
Copy link
Contributor Author

Thanks for taking the time to look at this pull request, John.

What's the motivation here? Why do you not have the coverage config file in the same directory that you're running nose?

I dislike the inconsistency of how different Python projects handle configuration files. Some projects by default use a visible config file, some use a hidden dotfile, others use setup.cfg, etc. In this case, coverage allows me to move the config file but nose doesn't currently pass that configuration forward.

Also, not much gets merged these days without tests.

I can sympathize. I looked into adding testing for this particular option but it didn't seem like there was significant testing for coverage configuration options to begin with. You can expect the next commit to have additional testing.

@jszakmeister
Copy link
Contributor

I dislike the inconsistency of how different Python projects handle configuration files. Some projects by default use a visible config file, some use a hidden dotfile, others use setup.cfg, etc. In this case, coverage allows me to move the config file but nose doesn't currently pass that configuration forward.

I see. You want to do it a particular way, and we stand in the way.

I can sympathize. I looked into adding testing for this particular option but it didn't seem like there was significant testing for coverage configuration options to begin with. You can expect the next commit to have additional testing.

That happens, yes. Nose has been around a while, and even though there a lots of tests, it still lacks in some areas. The project's standards have evolved over time, so some portions are lacking. Thank you for improving the situation.

@brian-reinhart
Copy link
Contributor Author

I see. You want to do it a particular way, and we stand in the way.

Thank you for understanding.

@jszakmeister jszakmeister added this to the 1.4.0 milestone Apr 4, 2015
@brian-reinhart brian-reinhart force-pushed the add_cover_config_file_option branch from 8f8c583 to 241af6b Compare April 11, 2015 22:30
@brian-reinhart brian-reinhart force-pushed the add_cover_config_file_option branch from 241af6b to 89687d0 Compare April 11, 2015 22:51
@brian-reinhart
Copy link
Contributor Author

Ping.

I just wanted to get an update on the status of this and if there is anything left for me to do.

@jszakmeister
Copy link
Contributor

Can you fix up the history to squash the relevant fix commits into the original ones? Sorry, but I'm a bit picky about that stuff, especially since I spend so much time tracking down bugs.

jszakmeister added a commit to jszakmeister/nose that referenced this pull request Nov 28, 2015
Adds an option to specify the location of the coverage config file.
@jszakmeister
Copy link
Contributor

Merged an amended version of this in 0f592c4.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants