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

Feat: add minimum save strategy #752

Merged
merged 8 commits into from
Nov 20, 2021

Conversation

agoose77
Copy link
Contributor

@agoose77 agoose77 commented Nov 19, 2021

Pull Request Check List

  • A news fragment is added in news/ describing what is new.
  • Test cases added for changed code.

Describe what you have changed in this PR.

This PR adds --save-minimum to pdm add for cases where the user doesn't want an upper bound (e.g. non SemVer uses)

See also

This strategy uses lower-bounds for version constraints.
This is useful for calver, where the upper-bound no longer
explicity defines API compatibility.
Co-authored-by: Timothée Mazzucotelli <pawamoy@pm.me>
@agoose77 agoose77 marked this pull request as ready for review November 19, 2021 19:23
@agoose77
Copy link
Contributor Author

agoose77 commented Nov 19, 2021

I just want to take the time to say how cool this project is. It's really easy to navigate the source code, and I like the fact that the scope of pdm is quite small. I don't use PEP 582, and it doesn't feel like I'm punished for that, which is also nice!

I was looking for something a bit more customiseable than poetry, and uses PEP 621, and pdm is perfect.

@agoose77 agoose77 changed the title Feat: add lb strategy Feat: add minimum save strategy Nov 19, 2021
@codecov-commenter
Copy link

codecov-commenter commented Nov 20, 2021

Codecov Report

Merging #752 (2a9d218) into main (bf4ea7a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #752   +/-   ##
=======================================
  Coverage   83.77%   83.77%           
=======================================
  Files          73       73           
  Lines        6143     6146    +3     
  Branches     1094     1095    +1     
=======================================
+ Hits         5146     5149    +3     
  Misses        705      705           
  Partials      292      292           
Flag Coverage Δ
unittests 83.56% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pdm/cli/options.py 96.42% <100.00%> (+0.04%) ⬆️
pdm/cli/utils.py 82.91% <100.00%> (+0.10%) ⬆️

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 bf4ea7a...2a9d218. Read the comment docs.

@frostming
Copy link
Collaborator

@agoose77 Welcome to PDM! Care to update the completion scripts under pdm/cli/completions? It is okay if you won't, I will do that instead.

@agoose77
Copy link
Contributor Author

@agoose77 Welcome to PDM! Care to update the completion scripts under pdm/cli/completions? It is okay if you won't, I will do that instead.

Darn, I thought I'd caught everything! Will do.

@agoose77
Copy link
Contributor Author

agoose77 commented Nov 20, 2021

@frostming I've updated the completions, and modified the completions script to support ZSH and powershell. Was this a deliberate omission, or am I OK to keep the fix in this PR? I can't test the PS1 changes without a Windows machine, and it seems like a big diff.

@frostming
Copy link
Collaborator

oh, just keep them and add a single option manually

@agoose77
Copy link
Contributor Author

agoose77 commented Nov 20, 2021

oh, just keep them and add a single option manually

OK, will revert the change. Out of interest, why do we not use the completions generator for powershell/zsh? I noticed some spelling errors in the hand-crafted ones.

@agoose77 agoose77 force-pushed the feat-add-lb-strategy branch from cb98569 to f24c420 Compare November 20, 2021 10:57
@frostming
Copy link
Collaborator

oh, just keep them and add a single option manually

OK, will revert the change. Out of interest, why do we not use the completions generator for powershell/zsh? I noticed some spelling errors in the hand-crafted ones.

because they are hand written, for some value completion. no autocomplete engine can generate such customized contents.

@agoose77
Copy link
Contributor Author

Right, makes sense. I think this PR is done from my end, are there any outstanding change to be made? :)

@frostming
Copy link
Collaborator

LGTM, thanks for fixing all the typos

@frostming frostming merged commit 0ec5c7e into pdm-project:main Nov 20, 2021
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.

4 participants