search

cool-RR / PySnooper

Never use print for debugging again
MIT License
16.37k stars 951 forks source link

Python 3.10 compatibility #219

Closed frenzymadness closed 3 years ago

frenzymadness commented 3 years ago

Fixes: #218

This is the minimum I had to implement to make the tests compatible with Python 3.10. I also did some improvements in configs for CI and tests.

I'm not sure what is your approach to flake8 and pylint because there are already many errors and it's hard to find the ones possible caused by me here. Also, it's kinda verbose to use the names for keyword arguments instead of **kwargs for the optional ones but I've followed the current codebase.

All unit tests are passing locally so we'll see what CI thinks about it.

cool-RR commented 3 years ago

This looks good to me. @alexmojaki Do you want to take a look?

A few notes:

  1. Anywhere you use (3,10), space it out: (3, 10)
  2. I don't like that the expected_entries variable gets replaced. Can you remove the assignment expected_entries = filtered_expected_entries and just iterate over filtered_expected_entries in the for loop?
  3. The CI is failing for some reason, but I guess you noticed that.
frenzymadness commented 3 years ago

Thanks for the feedback. I've tried to improve the logic to be more readable. See the latest fixup commit.

The CI fails because Travis has not the latest Python 3.10 beta 1 yet, it's still on alpha 5. If you want to, I can add the 3.10 job into allow_failures and we can remove it from there once Travis updates its Python.

cool-RR commented 3 years ago

Great job. Yes, using allow_failures sounds good to me.

On Tue, May 18, 2021 at 8:02 PM frenzymadness @.***> wrote:

Thanks for the feedback. I've tried to improve the logic to be more readable. See the latest fixup commit.

The CI fails because Travis has not the latest Python 3.10 beta 1 yet, it's still on alpha 5. If you want to, I can add the 3.10 job into allow_failures and we can remove it from there once Travis updates its Python.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/cool-RR/PySnooper/pull/219#issuecomment-843363280, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAN3SUIH7YF5OHVDZGE2LTTOKMKRANCNFSM45CQ2E2Q .

alexmojaki commented 3 years ago

No comments from me because I don't like the 'Entry' style tests anyway.

cool-RR commented 3 years ago

Hmm, any idea why Travis still fails?

frenzymadness commented 3 years ago

The configuration is kinda different when handled by the tox-travis, let me find the right way.

cool-RR commented 3 years ago

Okay, Travis is green. Are we good to merge? (After the merge I'll do a version bump and make a release.)

frenzymadness commented 3 years ago

I think we are. Thank you!

cool-RR commented 3 years ago

Done and release. Thank you.

frenzymadness commented 3 years ago

Thanks for the release. Could you please also create a git tag?

cool-RR commented 3 years ago

Right, done.

On Thu, May 20, 2021 at 10:24 AM frenzymadness @.***> wrote:

Thanks for the release. Could you please also create a git tag?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/cool-RR/PySnooper/pull/219#issuecomment-844793029, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAN3SWVZUCTTSCFNKSXEZ3TOS2JVANCNFSM45CQ2E2Q .