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

Drop python3.8 support #1003

Merged
merged 6 commits into from
Oct 23, 2024
Merged

Drop python3.8 support #1003

merged 6 commits into from
Oct 23, 2024

Conversation

RyanBalfanz
Copy link
Member

@RyanBalfanz RyanBalfanz commented Oct 23, 2024

Closes #

Python 3.8 EOL date was 2024-10-07.

See #824 for prior art, dropping Python 3.7.

💸 TL;DR

Drops Python v3.8 support.

📜 Details

Design Doc

Jira

🧪 Testing Steps / Validation

✅ Checks

  • CI tests (if present) are passing
  • Adheres to code style for repo
  • Contributor License Agreement (CLA) completed if not a Reddit employee

# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand.
# This file is automatically @generated by Poetry 1.8.2 and should not be changed by hand.
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: I installed the version that is installed explicitly in the Dockerfile, hence the version change.

@RyanBalfanz RyanBalfanz mentioned this pull request Oct 23, 2024
3 tasks
@RyanBalfanz RyanBalfanz marked this pull request as ready for review October 23, 2024 03:16
@RyanBalfanz RyanBalfanz requested a review from a team as a code owner October 23, 2024 03:16
Copy link

@mathyourlife-reddit mathyourlife-reddit left a comment

Choose a reason for hiding this comment

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

Looks like updates are consistent with the 3.7 drop and test are 🟢 . Seems reasonable to me, would also like a 👍 from @chriskuehl before merging.

@RyanBalfanz
Copy link
Member Author

RyanBalfanz commented Oct 23, 2024

Looks like updates are consistent with the 3.7 drop and test are 🟢 . Seems reasonable to me, would also like a 👍 from @chriskuehl before merging.

3.7 drop didn't require an updated poetry.lock but CI was failing without it, so I am keen to call that out here

Copy link
Member

@chriskuehl chriskuehl left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this!

@chriskuehl
Copy link
Member

3.7 drop didn't require an updated poetry.lock but CI was failing without it, so I am keen to call that out here

This looks good to me. Needing to update the poetry.lock is expected when you change the pyproject.toml since it contains a hash of the important fields, and also it may need to adjust the package versions if you change dependencies. You can use poetry lock --no-update to do a minimal update (should be fast as it only fixes the stuff it needs to), or poetry lock (which can be slow and pull in a ton of updates).

Worth saying explicitly that poetry.lock does not affect users of Baseplate in any way. It's only used for Baseplate's own tests and local development, and is not even included in the sdist or wheel. So, even if you run poetry lock and pull in more changes than necessary, it has no impact on users so there is zero risk here.

@chriskuehl chriskuehl merged commit fe59ab2 into reddit:develop Oct 23, 2024
4 checks passed
chriskuehl added a commit that referenced this pull request Nov 12, 2024
chriskuehl added a commit that referenced this pull request Nov 13, 2024
chriskuehl added a commit that referenced this pull request Nov 13, 2024
* Revert "Migrate to Ruff (#984)"

This reverts commit c5b7ca9.

* poetry lock --no-update

* Revert "Drop python3.8 support (#1003)"

This reverts commit fe59ab2.

* poetry lock --no-update

* Fix mypy errors

* Add future annotations import to test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants