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

Use click API for output file handling in pip-compile #727

Merged
merged 1 commit into from
Mar 27, 2019

Conversation

atugushev
Copy link
Member

@atugushev atugushev commented Jan 31, 2019

  • Use click's file option with atomic mode instead of ExitStack+AtomicSaver.
  • Less code, DRY.
  • Less untested code.

Changelog-friendly one-liner: Use click API for output file handling in pip-compile

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

@atugushev atugushev added the help wanted Request help from the community label Jan 31, 2019
@atugushev atugushev changed the title Use click API for output file handling in pip-compile WIP: Use click API for output file handling in pip-compile Jan 31, 2019
@codecov
Copy link

codecov bot commented Jan 31, 2019

Codecov Report

Merging #727 into master will increase coverage by 5.55%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #727      +/-   ##
=========================================
+ Coverage   92.34%   97.9%   +5.55%     
=========================================
  Files          35      34       -1     
  Lines        2285    1953     -332     
  Branches      308     254      -54     
=========================================
- Hits         2110    1912     -198     
+ Misses        147      29     -118     
+ Partials       28      12      -16
Impacted Files Coverage Δ
piptools/utils.py 100% <ø> (ø) ⬆️
piptools/scripts/compile.py 100% <100%> (ø) ⬆️
piptools/writer.py 100% <100%> (ø) ⬆️
tests/test_writer.py 100% <100%> (ø) ⬆️
tests/conftest.py 95.18% <100%> (+0.3%) ⬆️

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 e20ebac...964e8e7. Read the comment docs.

@atugushev
Copy link
Member Author

Hello @vphilippon. I'm just wondering whether it is worth efforts, or "don't touch if it works".

@vphilippon
Copy link
Member

@atugushev Thanks for that PR, I'm always interested in some cleanup/maintanabillity improvements.

  • Use click's file option with atomic mode instead of ExitStack+AtomicSaver.

Cool, let's get that in!

  • New feature: add support for stdout as output file, for example pip-compile -o- [...]

I'd suggest to take this one out of the current MR for now. The feature is pretty much unusable with the issue you've raised, so I wouldn't merge/release that yet.
I'm super happy about seeing work done to get this feature though, so you're more than welcome to open a second MR once this is merged to get the discussion going.

I'll get on the review proper soon:tm: too.

@vphilippon vphilippon added this to the 3.4.0 milestone Feb 6, 2019
@atugushev
Copy link
Member Author

@vphilippon Thank you for the feedback! Yes, that'd be better to split up the changes.

@atugushev atugushev changed the title WIP: Use click API for output file handling in pip-compile Use click API for output file handling in pip-compile Feb 13, 2019
@atugushev atugushev added refactor Refactoring code and removed help wanted Request help from the community labels Feb 13, 2019
@atugushev
Copy link
Member Author

Hello @vphilippon,

The PR is ready to review. If you have a time take a look please.

@atugushev
Copy link
Member Author

Conflicts are resolved.

@vphilippon vphilippon modified the milestones: 3.4.0, 3.5.0 Feb 19, 2019
@atugushev atugushev modified the milestones: 3.5.0, 3.6.0 Mar 11, 2019
@atugushev
Copy link
Member Author

@blueyed need your feedback :)

Copy link
Contributor

@blueyed blueyed left a comment

Choose a reason for hiding this comment

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

Sweet!

@atugushev
Copy link
Member Author

@blueyed thanks! I'll resolve the conflicts shortly.

@atugushev atugushev added this to the 3.6.0 milestone Mar 27, 2019
@atugushev atugushev merged commit b050a7f into jazzband:master Mar 27, 2019
@atugushev atugushev removed this from the 3.6.0 milestone Apr 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactoring code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants