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

Write out default index when it is provided as--extra-index-url #1325

Merged

Conversation

fahrradflucht
Copy link
Contributor

@fahrradflucht fahrradflucht commented Feb 15, 2021

If the default index is provided as --extra-index-url it can not safely be dropped from the written output since this changes behavior when the output is synced.

While #756 Explicitly introduced tests for the opposite, this seams to have been a mistake since otherwise the following lines comment about making an exception would be nonsensical.

Changelog-friendly one-liner: Write out default index when it is provided as --extra-index-url.

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

If the default index is provided as `--extra-index-url` it can not
safely be dropped from the written output since this changes behaviour
when the output is synced.

While jazzband#756 Explicitly introduced tests for the
opposite this seams to have been a mistake since otherwise the following
lines comment about making an exception would be nonsensical.
@fahrradflucht fahrradflucht changed the title write default index in "--extra-index-url" Write out default index when it is provided as--extra-index-url Feb 15, 2021
@codecov
Copy link

codecov bot commented Feb 15, 2021

Codecov Report

Merging #1325 (3ad0b05) into master (a5e257e) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1325      +/-   ##
==========================================
- Coverage   99.62%   99.62%   -0.01%     
==========================================
  Files          33       33              
  Lines        2921     2920       -1     
  Branches      318      318              
==========================================
- Hits         2910     2909       -1     
  Misses          5        5              
  Partials        6        6              
Impacted Files Coverage Δ
tests/test_writer.py 100.00% <ø> (ø)
piptools/writer.py 100.00% <100.00%> (ø)
piptools/repositories/pypi.py 97.25% <0.00%> (-0.02%) ⬇️

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 a5e257e...cbf89e6. Read the comment docs.

@fahrradflucht
Copy link
Contributor Author

@atugushev Since you wrote that test, did I interpret that right?

@atugushev atugushev added this to the 6.0.0 milestone Feb 19, 2021
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.

👍

@atugushev atugushev merged commit 2be5cbf into jazzband:master Feb 23, 2021
@atugushev
Copy link
Member

@fahrradflucht thanks for the contribution! 🎉

@atugushev atugushev added the writer Related to results output writer component label Feb 23, 2021
@fahrradflucht fahrradflucht deleted the fix/emit-extra-default-index-url branch March 6, 2021 19:00
@atugushev
Copy link
Member

pip-tools v6.0.0 is released 🚀

This was referenced Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
writer Related to results output writer component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants