-
Notifications
You must be signed in to change notification settings - Fork 38
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
MNT: Compat with pytest>=8 #236
Conversation
because the hook check has changed in 8.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find having the conditional around the class makes things less readable, but if you say it's better this way, I'm OK with it.
That's being said, in the else branch there are some cleanups to be made for superfluous conditional.
Oh, and please mention it in the changelog. |
as suggested by bsipocz
and add Dependabot for future auto-update
@bsipocz , I think I addressed your comments. Also did some infrastructure updates that I noticed while porting stuff from here to astropy/pytest-filter-subpackage#15 . |
to within class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code changes look, good, but I would like to play it save and include 8.0.x in the test suite. Given it's already the evening for you, I'll just push a commit to this branch, and go ahead and merge if CI is all green.
Thanks @pllim! |
Indeed I was AFK so thanks for wrapping this up! 🙇♀️ |
Because the hook check has changed in 8.1 . See pytest-dev/pytest#11779 (comment)