pylint-dev / pylint-pytest

A Pylint plugin to suppress pytest-related false positives.
https://pypi.org/project/pylint-pytest/
MIT License
15 stars 3 forks source link

Drop python 3.6 and 3.7 which are past end of life upstream #23

Closed Pierre-Sassoulas closed 10 months ago

Pierre-Sassoulas commented 11 months ago

Cherry-picked from #8, I'm doing things progressively so it's reviewable.

Closes https://github.com/pylint-dev/pylint-pytest/issues/11

codecov[bot] commented 11 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (48212e2) 94.16% compared to head (f6e9b9e) 93.97%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #23 +/- ## ========================================== - Coverage 94.16% 93.97% -0.19% ========================================== Files 18 18 Lines 548 548 Branches 106 106 ========================================== - Hits 516 515 -1 Misses 23 23 - Partials 9 10 +1 ``` | [Flag](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/23/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | Coverage Δ | | |---|---|---| | [3.10](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/23/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `93.97% <ø> (ø)` | | | [3.11](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/23/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `93.97% <ø> (ø)` | | | [3.6](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/23/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `?` | | | [3.7](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/23/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `?` | | | [3.8](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/23/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `93.97% <ø> (ø)` | | | [3.9](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/23/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `93.97% <ø> (ø)` | | | [macos-latest](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/23/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `93.97% <ø> (ø)` | | | [ubuntu-20.04](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/23/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `?` | | | [ubuntu-latest](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/23/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `93.97% <ø> (ø)` | | | [windows-latest](https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/23/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev) | `93.97% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pylint-dev#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Pierre-Sassoulas commented 11 months ago

@stdedos I think it could be merged even with the negative coverage change.

Pierre-Sassoulas commented 10 months ago

@stdedos could you review this, please :) ? The CI is red because of an indirect coverage changes that I think we can ignore.

stdedos commented 10 months ago

@stdedos could you review this, please :) ? The CI is red because of an indirect coverage changes that I think we can ignore.

Yeah, this seems to be the problem: image https://app.codecov.io/gh/pylint-dev/pylint-pytest/pull/23/indirect-changes

I'd be nice to figure out why is this happening, but it is definitely not a priority - given the benefits of this PR.

"I am not reviewing this" since it's not scheduled to go in yet. Otherwise all looks golden to me and it is approved 💚


Plan is:

Question:

  1. Can https://github.com/pylint-dev/pylint-pytest/pull/24 become thinner? I could consider it for v1
  2. Can https://github.com/pylint-dev/pylint-pytest/issues/12 be fixed for v1? I lost track on what was the pylint issue 😅
Pierre-Sassoulas commented 10 months ago

Can https://github.com/pylint-dev/pylint-pytest/pull/24 become thinner? I could consider it for v1

We can remove the drop of python 3.6 / 3.7 (probably)

Can https://github.com/pylint-dev/pylint-pytest/issues/12 be fixed for v1? I lost track on what was the pylint issue 😅

Need to support 3.12 maybe something else

stdedos commented 10 months ago

Can #24 become thinner? I could consider it for v1

We can remove the drop of python 3.6 / 3.7 (probably)

I'd consider it for v1 - but it doesn't have to wait "the usual" 1w-period.

So next week we release master, and a thin #24 afterwards

Pierre-Sassoulas commented 10 months ago

Hey, I tried dropping the removal of python 3.6 / 3.7 and simply adding python 3.12, but this is non trivial (https://github.com/pylint-dev/pylint-pytest/actions/runs/7010519218/job/19071209667?pr=24). It's been nearly two months since the release of python 3.12 / pylint 3.0 and python 3.6 / 3.7 have been EOL for a long time, maybe we could just drop EOL interpreters and release 2.X serie, what do you think ?

Pierre-Sassoulas commented 10 months ago

See this issue in vscode/pylint : https://github.com/microsoft/vscode-pylint/issues/460 for example

stdedos commented 10 months ago

Thank you for trying ❤️

Sadly, my OSS time has been near-zero these days. I will make it my first priority to merge these.

Pierre-Sassoulas commented 10 months ago

No problem. This one is ready the coverage drop is acceptable imo.

stdedos commented 10 months ago

I took the liberty to prepare this for tomorrow 🙃

Pierre-Sassoulas commented 10 months ago

Thank you @stdedos :)

stdedos commented 10 months ago

Thank you @Pierre-Sassoulas for all your contributions and patience 😁 🙏