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

Local repository needs to notify proxied repo of ireq copying #1159

Merged
merged 1 commit into from
Jun 9, 2020

Conversation

richafrank
Copy link
Contributor

@richafrank richafrank commented Jun 6, 2020

Otherwise, the proxied repository loses track of what dependencies
the copy has. The local repo is used to prefer versions from an
existing requirements file.

Fixes #1154.
Fixes #1155.

Changelog-friendly one-liner: Fixed copying of dependencies when requirements are combined and there is an existing requirements file.

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).

@codecov
Copy link

codecov bot commented Jun 6, 2020

Codecov Report

Merging #1159 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1159   +/-   ##
=======================================
  Coverage   99.50%   99.50%           
=======================================
  Files          36       36           
  Lines        2802     2836   +34     
  Branches      331      332    +1     
=======================================
+ Hits         2788     2822   +34     
  Misses          8        8           
  Partials        6        6           
Impacted Files Coverage Δ
piptools/repositories/base.py 100.00% <100.00%> (ø)
piptools/repositories/local.py 96.36% <100.00%> (+0.13%) ⬆️
tests/conftest.py 100.00% <100.00%> (ø)
tests/test_cli_compile.py 100.00% <100.00%> (ø)
tests/test_repository_local.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 7ba96db...1c65e55. Read the comment docs.

@richafrank
Copy link
Contributor Author

Let me know if you think more of an integration test is worthwhile.

Copy link
Member

@atugushev atugushev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this! Could we have an integration test for this regression?

Otherwise, the proxied repository loses track of what dependencies
the copy has. The local repo is used to prefer versions from an
existing requirements file.
@richafrank richafrank force-pushed the recompile-drops-dependencies branch from 6e8dd89 to 1c65e55 Compare June 7, 2020 17:15
@richafrank
Copy link
Contributor Author

@atugushev 👍 Added

Copy link
Member

@atugushev atugushev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the tests! I would use make_package fixture though instead of using existing wheels (to isolate tests), but we could do it in following up PR.

@atugushev atugushev added this to the 5.2.1 milestone Jun 9, 2020
@atugushev atugushev merged commit 0d05314 into jazzband:master Jun 9, 2020
@richafrank richafrank deleted the recompile-drops-dependencies branch June 9, 2020 12:15
richafrank added a commit to richafrank/pip-tools that referenced this pull request Jun 12, 2020
atugushev pushed a commit that referenced this pull request Jun 13, 2020
JiachenSmith added a commit to JiachenSmith/pip-tools that referenced this pull request Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants