Skip to content

StableRequestCheck: Add stabletime config option#429

Closed
EmRowlands wants to merge 2 commits intopkgcore:masterfrom
EmRowlands:add-stabletime
Closed

StableRequestCheck: Add stabletime config option#429
EmRowlands wants to merge 2 commits intopkgcore:masterfrom
EmRowlands:add-stabletime

Conversation

@EmRowlands
Copy link
Copy Markdown

This commit adds an option allows the user to specify the time before a version is flagged by StableRequestCheck. The primary use case for this is in overlays where the stabilisation policy may be different from ::gentoo.

I deliberately did not add a short option for --stabletime because there are a limited number of valid single-letter options, and I didn't think that this case warranted using one. If maintainers think it's a good idea to have one, I'd be happy to add it in.

@thesamesam
Copy link
Copy Markdown
Member

thesamesam commented Aug 13, 2022

Seems reasonable. Might be nice to see if we can add a test for it.

@EmRowlands
Copy link
Copy Markdown
Author

Instead of adding a new test, I've added a fixture to an existing test that runs with varying values of --stabletime. The existing test case is preserved by including a case that uses a default value.

This solution might be a bit over-engineering-y, so I'm happy to split the default-value case back out into a separate test and use pytest.mark.parametrize instead. I decided to use this way round because it allows for other tests to use it in future.

Copy link
Copy Markdown
Member

@arthurzam arthurzam left a comment

Choose a reason for hiding this comment

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

@EmRowlands Could you please add a signoff to the commit (to the same rules as for Gentoo repository)?
While this isn't a must policy, I do plan to move the whole stack to Gentoo infra repo, and having a signoff would make our life easier.


Also, this looks very nice, so I do plan to merge it after those comments. In which case during the release commit, I need to write your contribution in the NEWS file with your name (as a thanks token) - so which is the preferred way to write your full name?

Comment thread src/pkgcheck/scripts/pkgcheck_scan.py
Comment thread tests/checks/test_stablereq.py Outdated
Comment thread tests/checks/test_stablereq.py Outdated
@arthurzam arthurzam self-assigned this Aug 13, 2022
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 13, 2022

Codecov Report

Merging #429 (b51b431) into master (0fb19f6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #429   +/-   ##
=======================================
  Coverage   95.96%   95.96%           
=======================================
  Files          55       55           
  Lines        7813     7814    +1     
  Branches     2211     2211           
=======================================
+ Hits         7498     7499    +1     
  Misses        194      194           
  Partials      121      121           
Impacted Files Coverage Δ
src/pkgcheck/checks/stablereq.py 98.03% <100.00%> (ø)
src/pkgcheck/scripts/pkgcheck_scan.py 100.00% <100.00%> (ø)

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

@EmRowlands
Copy link
Copy Markdown
Author

Could you please add a signoff to the commit (to the same rules as for Gentoo repository)? While this isn't a must policy, I do plan to move the whole stack to Gentoo infra repo, and having a signoff would make our life easier.

Last I checked, I was unable to contribute to Gentoo itself because I ran afoul of one of the relevant GLEPs. I'll have a read through of the notes I made after I push the changes you've suggested, and I'll push again later with the signoff if there's no issues.

Also, this looks very nice, so I do plan to merge it after those comments. In which case, I need to write your contribution in the NEWS file with your name (as a thanks token) - so which is the preferred way to write your full name?

Thank you, I appreciate the gesture. My full name can be written as it appears in the git log: "Emily Rowlands".

@EmRowlands
Copy link
Copy Markdown
Author

I've implemented the suggested changes, but (as I said in #429 (comment)) I've not added the signoff.

I'll report back once I've investigated why it was that I was previously unable to do it.

@thesamesam
Copy link
Copy Markdown
Member

Please feel free to email me privately (or arthur) if needed.

@EmRowlands
Copy link
Copy Markdown
Author

(force-push again because I messed up my rebase)

This commit adds an option allows the user to specify the time
before a version is flagged by StableRequestCheck. The primary use
case for this is in overlays where the stabilisation policy may be
different from ::gentoo.

Signed-off-by: Emily Rowlands <emily@erowl.net>
This commit parametrizes test_existing_stable_keyword. The existing
test case is preserved by including a case that uses a default
value.

Signed-off-by: Emily Rowlands <emily@erowl.net>
@EmRowlands
Copy link
Copy Markdown
Author

Good news: the situation that prevented me from doing the signoff no longer applies.

Please re-review or merge if ready!

@EmRowlands EmRowlands requested a review from arthurzam August 13, 2022 21:17
@arthurzam arthurzam closed this in b11d8c1 Aug 14, 2022
@arthurzam
Copy link
Copy Markdown
Member

Also, this looks very nice, so I do plan to merge it after those comments. In which case, I need to write your contribution in the NEWS file with your name (as a thanks token) - so which is the preferred way to write your full name?

Thank you, I appreciate the gesture. My full name can be written as it appears in the git log: "Emily Rowlands".

@EmRowlands, as agreed, you can see your name in NEWS file, and in v0.10.13 release notes
Thank you very much for the contribution, and we would be thankful for any future contributions!

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.

3 participants