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 _add_defaults_data_files override and add unittest #843

Merged
merged 1 commit into from Dec 1, 2016

Conversation

@tweksteen
Copy link
Contributor

@tweksteen tweksteen commented Nov 8, 2016

Reopening a pull request similar to PR #830. I've included unit test for that specific feature.

The only noticeable package that will fail with this patch is django_compressor because of dependencies on rcssmin and rjsmin. The main author has been contacted and is aware of the issue. It is best to organise a meaningful timeline with him to fix his issues before merging this PR.

@jaraco
Copy link
Member

@jaraco jaraco commented Nov 18, 2016

@tweksteen: Would you consider augmenting this pull request to specifically detect the case where non-standard values are being passed for data_files, log an error, and omit the files? That way, for this rare case, setuptools will continue to work as it did before, but for everything else it will work fine. The warning message will also provide incentive to the offending packages to correct their behavior.

@tweksteen tweksteen force-pushed the tweksteen:pypi_tests branch from 3494d62 to 0235d8e Nov 20, 2016
@tweksteen tweksteen force-pushed the tweksteen:pypi_tests branch from 0235d8e to 11b921f Nov 20, 2016
@tweksteen
Copy link
Contributor Author

@tweksteen tweksteen commented Nov 20, 2016

Agreed and done. Let me know if the warning message is clear enough. (Also, should self.warn, log.warn and warning.warn be merged somehow?)

@jaraco
Copy link
Member

@jaraco jaraco commented Dec 1, 2016

Looks good. And after fixing the test failures due to other flux in the ecosystem, the PR tests are passing. I'm prepared to accept.

@jaraco jaraco merged commit e846e06 into pypa:master Dec 1, 2016
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
jaraco added a commit that referenced this pull request Dec 1, 2016
@jaraco
Copy link
Member

@jaraco jaraco commented Dec 1, 2016

I spoke too soon. The tests for Python 3.7 are failing.

jaraco added a commit that referenced this pull request Dec 1, 2016
…n Python 2 where old style classes are used. Ref #843.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants