Skip to content

Conversation

agoose77
Copy link

@agoose77 agoose77 commented Mar 24, 2023

I didn't bisect to find when 3.10 adds the slots argument; I know it's in the release itself, and I don't think we need to be that careful to provide features to old alpha/beta/rc releases.

@welcome
Copy link

welcome bot commented Mar 24, 2023

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

@codecov
Copy link

codecov bot commented Mar 24, 2023

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.07%. Comparing base (e717248) to head (7262cbe).
⚠️ Report is 54 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@ ## master #258 +/- ## ======================================= Coverage 96.07% 96.07% ======================================= Files 62 62 Lines 3236 3236 ======================================= Hits 3109 3109 Misses 127 127 
Flag Coverage Δ
pytests 96.07% <100.00%> (ø)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
@chrisjsewell
Copy link
Member

Thanks @agoose77, @hukkin wrote this particular part of the code, and so I would suggest he should have a say on this change before merging

Copy link
Contributor

@hukkin hukkin left a comment

Choose a reason for hiding this comment

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

This should work, although my opinion is that support for alpha/beta releases should not be expected, so perhaps not worth it to further complicate the tuple comparison with non-integer values?

Co-authored-by: Taneli Hukkinen <3275109+hukkin@users.noreply.github.com>
@agoose77
Copy link
Author

agoose77 commented Apr 3, 2023

perhaps not worth it to further complicate the tuple comparison

I agree with the principle, but in this case I feel that the extra code is not that much more complex to justify not making this change? I'll defer to your final judgement though — merge this if you're happy, close if not! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants