[REM] pylint_odoo: Remove requirements/requirements.txt from build #499
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Could you check the red CID status, please? |
|
I'm not going to be super useful here as I don't know anything about pytest. Doesn't appear to be related though, looks like it's complaining about some flag you're passing to the code coverage tool? |
When installing a wheel the file `<installroot>/requirements/requirements.txt` is created. This appears to have been intentionally introduced to fix OCA#440 by introducing the following lines to `setup.cfg`: - https://github.com/OCA/pylint-odoo/blob/6e857c52117ef8597e0433b32ca6fac302ea76b8/setup.cfg#L38-L40 However, this is not the correct format for adding files (they should be an array of `(path, file)` tuples), resulting in a directory called "/requirements" that will contain the file, likely not what we want. It is unclear to me how source tarballs were being generated before, because requirements.txt should always be included when building with setuptools. **To Reproduce** Steps to reproduce the behavior: ``` $ python -m venv venv $ source venv/bin/activate $ pip install build installer wheel … $ python -m build --wheel … $ python -m installer dist/pylint_odoo-9.1.3-py3-none-any.whl $ ls venv/requirements/ requirements.txt ``` This is especially a problem for downstream OS packagers who will be installing the package to the (virtual) root directory as you won't want your final OS package creating `/requirements`. **Expected behavior** No files are installed outside of the normal filesystem. As far as I can tell this configuration can be removed from setup.cfg entirely. Alternatively, using package data is likely what was actually desired here (though, again, I don't see how it would be necessary in this case, building without this config results in requirements.txt being included for me): ``` [options.package_data] pylint_odoo = requirements.txt ``` Fix OCA#498
Remove the warning: - "This is unnecessary as this plugin provides the more complete --cov-context option."
moylop260
force-pushed
the
rm-build-req-moy
branch
from
f740cbe to
9c22f2b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When installing a wheel the file
<installroot>/requirements/requirements.txtis created. This appears to have been intentionally introduced to fix #440 by introducing the following lines tosetup.cfg:pylint-odoo/setup.cfg
Lines 38 to 40 in 6e857c5
However, this is not the correct format for adding files (they should be an array of
(path, file)tuples), resulting in a directory called "/requirements" that will contain the file, likely not what we want.It is unclear to me how source tarballs were being generated before, because requirements.txt should always be included when building with setuptools.
To Reproduce
Steps to reproduce the behavior:
This is especially a problem for downstream OS packagers who will be installing the package to the (virtual) root directory as you won't want your final OS package creating
/requirements.Expected behavior
No files are installed outside of the normal filesystem.
As far as I can tell this configuration can be removed from setup.cfg entirely. Alternatively, using package data is likely what was actually desired here (though, again, I don't see how it would be necessary in this case, building without this config results in requirements.txt being included for me):
Fix #498
Credits to @SamWhited