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

Fixing issue with losing data about local files #1385

Conversation

microcat49
Copy link
Contributor

Fixes the loss of information in resolver where we were not keeping track of the InstallRequirements local file information.

Contributor checklist
  • Provided the tests for the changes.
  • Gave a clear one-line description in the PR (that the maintainers can add to CHANGELOG.md on release).
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

@microcat49 microcat49 force-pushed the 1054_pip-compile_does_not_work_for_vsphere-automation-sdk-python_setup.py_on_GitHub branch 2 times, most recently from d8c090e to d1c6629 Compare April 15, 2021 02:43
@codecov
Copy link

codecov bot commented Apr 15, 2021

Codecov Report

Merging #1385 (8f5ca72) into master (3f19f8c) will decrease coverage by 0.19%.
The diff coverage is 68.42%.

❗ Current head 8f5ca72 differs from pull request most recent head 274cf5f. Consider uploading reports for the commit 274cf5f to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1385      +/-   ##
==========================================
- Coverage   99.67%   99.47%   -0.20%     
==========================================
  Files          33       33              
  Lines        3037     3055      +18     
  Branches      327      330       +3     
==========================================
+ Hits         3027     3039      +12     
- Misses          5        8       +3     
- Partials        5        8       +3     
Impacted Files Coverage Δ
piptools/resolver.py 95.83% <0.00%> (-3.55%) ⬇️
tests/conftest.py 100.00% <100.00%> (ø)
tests/constants.py 100.00% <100.00%> (ø)
tests/test_resolver.py 100.00% <100.00%> (ø)

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 3f19f8c...274cf5f. Read the comment docs.

When a dependency is a local file we need to keep track of that information. During the combine_install_requirements function we were losing that information. This merge request looks for that information and combines it into the InstallRequirements object that is going to be returned by that function.
@microcat49 microcat49 force-pushed the 1054_pip-compile_does_not_work_for_vsphere-automation-sdk-python_setup.py_on_GitHub branch from 064baf9 to b6db3f9 Compare April 15, 2021 03:26
@microcat49
Copy link
Contributor Author

@atugushev sorry about the delayed response. I think the 3.10 python test failure is due to something that is not related to my code. This PR is ready for review.

@azaghal
Copy link

azaghal commented Jun 22, 2021

Is there anything that could be done to get this pull request moving forward?

@AndydeCleyre
Copy link
Contributor

Thanks!

Can you please rebase this onto latest master? And if this is intended to fix #1054, please add such a note to the top post.

@AndydeCleyre
Copy link
Contributor

@microcat49

Are you still available to work on this? Or should I take your work (author-preserved) to a new PR for further work?

@adaamz
Copy link

adaamz commented Nov 23, 2021

ping @microcat49

@AndydeCleyre
Copy link
Contributor

I'm going to close this in favor of focusing related attention on #1519. Thank you for this, and please let me know if something's amiss!

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.

None yet

4 participants