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 "tests" package from sdist installation #120

Closed
wants to merge 2 commits into from

Conversation

kwshi
Copy link
Contributor

@kwshi kwshi commented Apr 30, 2021

Resolves #69 (nice). Currently the tomlkit tests are included in the distribution via packages, but that behavior is incorrect since it causes tests to be exposed (and installed!) as a module. We don't want that; we only want tests to be included in the source distribution, so the more sensible configuration is to use the include key.

Solution comes from this comment, as well as @abn's implementation of the same fix for Poetry.

@codecov-commenter
Copy link

codecov-commenter commented Apr 30, 2021

Codecov Report

Merging #120 (1aa1ec1) into master (d34a1fe) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #120   +/-   ##
=======================================
  Coverage   91.80%   91.80%           
=======================================
  Files          11       11           
  Lines        2391     2391           
=======================================
  Hits         2195     2195           
  Misses        196      196           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d34a1fe...1aa1ec1. Read the comment docs.

@kwshi kwshi force-pushed the rm-tests-package branch from 57eb268 to edefd8b Compare April 30, 2021 02:31
@kwshi kwshi force-pushed the rm-tests-package branch from edefd8b to ce48f64 Compare April 30, 2021 02:38
@eli-schwartz
Copy link

It is a teeny bit disappointing this fix did not get merged in time for 0.7.1, released 2 hours ago. Hopefully it can be merged in time for any future 0.7.2?

@frostming frostming closed this Dec 17, 2021
@kwshi
Copy link
Contributor Author

kwshi commented Dec 17, 2021

For reference, this PR was closed because I think the issue was fixed in 63b96f1.

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.

setup.py installs the tests package
4 participants