Always obey --upgrade-package, and allow it together with --upgrade #831

adamchainz · 2019-05-30T07:22:14Z

Fixes #829. Closes #830.

Allow the two options together, essentially making --upgrade-package a way of passing an extra constraint. Always parse --upgrade-package, regardless of whether the output file exists.

Changelog-friendly one-liner:

--upgrade and --upgrade-package are no longer mutually exclusive.
--upgrade-package now works even if the output file doesn't exist.

Contributor checklist

Provided the tests for the changes.
Requested a review from another contributor.
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).

adamchainz · 2019-05-30T07:23:34Z

As an external contributor, I can't request a reviewer or assign to a milestone 🙊

atugushev · 2019-05-30T07:23:45Z

Thanks for the contribution! I would suggest to split it to two PR, since #830 needs to be discussed.

adamchainz · 2019-05-30T07:24:54Z

I will split if anyone finds the proposal offensive but I think it's quite an easy win

atugushev · 2019-05-30T07:31:16Z

I will split if anyone finds the proposal offensive but I think it's quite an easy win

At least it would be much easier to review for maintainers.

Fixes jazzband#829. Fixes jazzband#830. Allow the two options together, essentially making `--upgrade-package` a way of passing an extra constraint. Always parse `--upgrade-package`, regardless of whether the output file exists.

codecov · 2019-05-30T07:39:28Z

Codecov Report

Merging #831 into master will decrease coverage by 0.11%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #831      +/-   ##
==========================================
- Coverage    98.8%   98.69%   -0.12%     
==========================================
  Files          36       36              
  Lines        2184     2139      -45     
  Branches      280      279       -1     
==========================================
- Hits         2158     2111      -47     
- Misses         15       16       +1     
- Partials       11       12       +1

Impacted Files	Coverage Δ
tests/test_cli_compile.py	`100% <100%> (ø)`	⬆️
piptools/scripts/compile.py	`100% <100%> (ø)`	⬆️
piptools/repositories/pypi.py	`92.63% <0%> (-1.22%)`	⬇️
piptools/repositories/local.py	`90.69% <0%> (-0.8%)`	⬇️
tests/conftest.py	`94.44% <0%> (-0.68%)`	⬇️
piptools/scripts/sync.py	`100% <0%> (ø)`	⬆️
piptools/resolver.py	`100% <0%> (ø)`	⬆️
tests/test_writer.py	`100% <0%> (ø)`	⬆️
piptools/cache.py	`100% <0%> (ø)`	⬆️
... and 12 more

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 4438d98...c1e79b6. Read the comment docs.

adamchainz · 2019-05-30T07:42:23Z

The fix for #830 is just the removal of the constraint on line 228 of compile.py. The actual behavioural fix is done by the move of code that fixes #829. If I split it will really mean branch-on-branch for #830 which isn't better is it?

atugushev · 2019-05-30T08:01:51Z

The fix for #830 is just the removal of the constraint on line 228 of compile.py. The actual behavioural fix is done by the move of code that fixes #829. If I split it will really mean branch-on-branch for #830 which isn't better is it?

It would be better if this two issues depended on each other. But they look like not depended and can be fixed(implemented) separately. I must emphasise it's not mandatory, just a simple recommendation :)

Have you researched why the options --upgrade and --upgrade-package are mutually exclusive by a deliberate design decision?

atugushev · 2019-05-30T08:24:27Z

It would be better if this two issues depended on each other. But they look like not depended and can be fixed(implemented) separately. I must emphasise it's not mandatory, just a simple recommendation :)

Oh, sorry, they should be fixed by one shot of course.

tests/test_cli_compile.py

adamchainz · 2019-05-30T09:09:35Z

Have you researched why the options --upgrade and --upgrade-package are mutually exclusive by a deliberate design decision?

It doesn't seem there was too much deliberation in #409 which added --upgrade-package and added the mutual exclusivity. I think it was just easier at the time given the way the code was laid out then.

adamchainz

Thanks for review! I decided to un-parameterize the tests since the branching was reducing readability :)

atugushev

Awesome!

adamchainz · 2019-06-06T13:26:21Z

Thanks @atugushev ! 🎉

atugushev · 2019-06-06T13:38:23Z

@adamchainz thanks for great work! I'll prepare a release within 24h.

* Move and duplicate note about running under multiple Python versions to the start of each command's section. * Add note about using `--upgrade` and `--upgrade-package` together following jazzband#831. * Document `--output-file`.

atugushev · 2019-06-06T20:06:29Z

pip-tools v3.8.0 is released

* Move and duplicate note about running under multiple Python versions to the start of each command's section. * Add note about using `--upgrade` and `--upgrade-package` together following #831. * Document `--output-file`.

adamchainz mentioned this pull request May 30, 2019

pip-compile --upgrade-package doesn't apply when requirements.txt doesn't exist #829

Closed

adamchainz force-pushed the issues_829_830 branch from 7cf6873 to 1a78dd2 Compare May 30, 2019 07:39

This comment has been minimized.

Sign in to view

atugushev added the bug fix label May 30, 2019

atugushev reviewed May 30, 2019

View reviewed changes

tests/test_cli_compile.py Show resolved Hide resolved

atugushev added the enhancement Improvements to functionality label May 30, 2019

atugushev reviewed May 30, 2019

View reviewed changes

tests/test_cli_compile.py Outdated Show resolved Hide resolved

atugushev reviewed May 30, 2019

View reviewed changes

tests/test_cli_compile.py Outdated Show resolved Hide resolved

Split out tests

c1e79b6

adamchainz commented May 30, 2019

View reviewed changes

atugushev approved these changes May 30, 2019

View reviewed changes

atugushev added this to the 3.8.0 milestone May 30, 2019

atugushev requested a review from blueyed May 30, 2019 11:44

adamchainz mentioned this pull request Jun 5, 2019

pip-compile --upgrade and --upgrade-package needn't be mutually exclusive #830

Closed

atugushev merged commit 0c78c0e into jazzband:master Jun 6, 2019

adamchainz deleted the issues_829_830 branch June 6, 2019 13:26

adamchainz mentioned this pull request Jun 6, 2019

Improve documentation #833

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Always obey --upgrade-package, and allow it together with --upgrade #831

Always obey --upgrade-package, and allow it together with --upgrade #831

adamchainz commented May 30, 2019 •

edited by atugushev

Loading

adamchainz commented May 30, 2019

atugushev commented May 30, 2019 •

edited

Loading

adamchainz commented May 30, 2019

atugushev commented May 30, 2019

This comment has been minimized.

codecov bot commented May 30, 2019 •

edited

Loading

adamchainz commented May 30, 2019

atugushev commented May 30, 2019

atugushev commented May 30, 2019

adamchainz commented May 30, 2019

adamchainz left a comment

atugushev left a comment

adamchainz commented Jun 6, 2019

atugushev commented Jun 6, 2019 •

edited

Loading

atugushev commented Jun 6, 2019

Always obey --upgrade-package, and allow it together with --upgrade #831

Always obey --upgrade-package, and allow it together with --upgrade #831

Conversation

adamchainz commented May 30, 2019 • edited by atugushev Loading

Contributor checklist

adamchainz commented May 30, 2019

atugushev commented May 30, 2019 • edited Loading

adamchainz commented May 30, 2019

atugushev commented May 30, 2019

This comment has been minimized.

codecov bot commented May 30, 2019 • edited Loading

Codecov Report

adamchainz commented May 30, 2019

atugushev commented May 30, 2019

atugushev commented May 30, 2019

adamchainz commented May 30, 2019

adamchainz left a comment

Choose a reason for hiding this comment

atugushev left a comment

Choose a reason for hiding this comment

adamchainz commented Jun 6, 2019

atugushev commented Jun 6, 2019 • edited Loading

atugushev commented Jun 6, 2019

adamchainz commented May 30, 2019 •

edited by atugushev

Loading

atugushev commented May 30, 2019 •

edited

Loading

codecov bot commented May 30, 2019 •

edited

Loading

atugushev commented Jun 6, 2019 •

edited

Loading