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

limit number of versions to keep only after filtering #48

Merged
merged 4 commits into from
Feb 12, 2023

Conversation

artazar
Copy link

@artazar artazar commented Oct 28, 2022

This PR changes the logic of 'keep_at_least' parameter.

Previously, it cut off the first n versions before applying any other conditions, so it was a global effect.

Now, first - the whole logic of filtering applies, including 'filter_tag', 'skip_tag', 'cut_off', and for the resulting set - we additionally skip n versions from the top of the list to be sure they persist.

Fixes #47

@codecov
Copy link

codecov bot commented Oct 28, 2022

Codecov Report

Merging #48 (b1ae540) into main (108bfb6) will increase coverage by 0.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #48      +/-   ##
==========================================
+ Coverage   90.17%   90.26%   +0.08%     
==========================================
  Files           1        1              
  Lines         224      226       +2     
  Branches       49       50       +1     
==========================================
+ Hits          202      204       +2     
  Misses         16       16              
  Partials        6        6              
Impacted Files Coverage Δ
main.py 90.26% <100.00%> (+0.08%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@sondrelg sondrelg left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good to me 👍 Could you please add a test for the new functionality?

@artazar
Copy link
Author

artazar commented Oct 28, 2022

I did check the test, and it appears already existing one covers it, no controversies appear.
But I will think about adding some extra test.

@sondrelg sondrelg merged commit 02b1c7a into snok:main Feb 12, 2023
@sondrelg
Copy link
Member

Thanks for the PR @artazar. I'll do some testing with the code currently in main, then hope to release a new major version shortly 🙂 Sorry for the delay

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.

Apply keep-at-least on the filtered tags list
2 participants