Skip to content
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

Pin pylint in setup.py #197

Merged
merged 2 commits into from
Apr 17, 2024
Merged

Pin pylint in setup.py #197

merged 2 commits into from
Apr 17, 2024

Conversation

luandy64
Copy link
Contributor

Description of change

Here's the first failing CI run
https://app.circleci.com/pipelines/github/singer-io/tap-stripe/2321/workflows/a7a108fe-1dbc-4b9a-ba70-8a540e793990/jobs/17567

Successfully installed astroid-3.1.0 backoff-2.2.1 certifi-2024.2.2 charset-normalizer-3.3.2 ciso8601-2.3.1 coverage-7.4.3 dill-0.3.8 idna-3.6 isort-5.13.2 jsonschema-2.6.0 mccabe-0.7.0 nose2-0.14.1 platformdirs-4.2.0 pylint-3.1.0

Here's the last good run
https://app.circleci.com/pipelines/github/singer-io/tap-stripe/2312/workflows/ad468452-6b8a-4e2e-971d-4cdc063f1228/jobs/17439

Successfully installed astroid-3.0.3 backoff-2.2.1 certifi-2024.2.2 charset-normalizer-3.3.2 ciso8601-2.3.1 coverage-7.4.1 dill-0.3.8 idna-3.6 isort-5.13.2 jsonschema-2.6.0 mccabe-0.7.0 nose2-0.14.1 platformdirs-4.2.0 pylint-3.0.3

Here's the new lint error we see

************* Module tap_stripe
tap_stripe/__init__.py:1058:12: R1731: Consider using 'max_created = max(max_created, created)' instead of unnecessary if block (consider-using-max-builtin)

-----------------------------------
Your code has been rated at 9.98/10

Manual QA steps

  • None, letting CI try this out

Risks

  • Low. Linting doesn't catch that many bugs and this is not an interesting rule to begin with

Rollback steps

  • revert this branch

Here's the first failing CI run
https://app.circleci.com/pipelines/github/singer-io/tap-stripe/2321/workflows/a7a108fe-1dbc-4b9a-ba70-8a540e793990/jobs/17567

> Successfully installed astroid-3.1.0 backoff-2.2.1 certifi-2024.2.2 charset-normalizer-3.3.2 ciso8601-2.3.1 coverage-7.4.3 dill-0.3.8 idna-3.6 isort-5.13.2 jsonschema-2.6.0 mccabe-0.7.0 nose2-0.14.1 platformdirs-4.2.0 pylint-3.1.0 

Here's the last good run
https://app.circleci.com/pipelines/github/singer-io/tap-stripe/2312/workflows/ad468452-6b8a-4e2e-971d-4cdc063f1228/jobs/17439

> Successfully installed astroid-3.0.3 backoff-2.2.1 certifi-2024.2.2 charset-normalizer-3.3.2 ciso8601-2.3.1 coverage-7.4.1 dill-0.3.8 idna-3.6 isort-5.13.2 jsonschema-2.6.0 mccabe-0.7.0 nose2-0.14.1 platformdirs-4.2.0 pylint-3.0.3
@luandy64
Copy link
Contributor Author

Digging into the changes a bit: I don't see how this was enabled between v3.0.3 and v3.1.0.
pylint-dev/pylint@v3.0.3...v3.1.0

You see consider-using-max-builtin pop up a few times in the diff, but it's mostly comments and test changes.

And the changelog page doesn't mention this rule at all: https://pylint.readthedocs.io/en/latest/whatsnew/3/3.1/index.html

@luandy64 luandy64 merged commit 593ae9f into master Apr 17, 2024
6 of 7 checks passed
@luandy64 luandy64 deleted the fix-pylint branch April 17, 2024 15:01
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.

1 participant