8000 removed checks meant for python version less then 3.6. by auvipy · Pull Request #19432 · django/django · GitHub
[go: up one dir, main page]
Skip to content

removed checks meant for python version less then 3.6. #19432

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

removed checks meant for python version less then 3.6. #19432

wants to merge 5 commits into from

Conversation

auvipy
Copy link
Contributor
@auvipy auvipy commented Apr 29, 2025 •
edited
Loading

Trac ticket number

"N/A"

Branch description

The code checks seems to be only for python version < 3.6. As django now support python version way above 3.6, I thought it might be a good idea to remove them now.

Trying to contribute to django after long time. so trying to familiar myself with new best practices of patch contributions!

Checklist

  • This PR targets the main branch.
  • The commit message is written in past tense, mentions the ticket number, and ends with a period.
  • I have checked the "Has patch" ticket flag in the Trac system.
  • I have added or updated relevant tests.
  • I have added or updated relevant docs, including release notes if applicable.

@github-actions github-actions bot added the no ticket Based on PR title, no linked Trac ticket label Apr 29, 2025
@auvipy auvipy changed the title remove checks meant for python version less then 3.6 removed checks meant for python version less then 3.6. Apr 29, 2025
@auvipy auvipy marked this pull request as ready for review April 29, 2025 11:41
@sarahboyce
Copy link
Contributor

This reverts the fix for ticket-27138
I believe that this is still valid as the docs still say that localtime does not to accept naive datetimes (https://docs.djangoproject.com/en/5.2/ref/utils/#django.utils.timezone.localtime)
So I'm not sure we should remove this 🤔

@auvipy
Copy link
Contributor Author
auvipy commented Apr 29, 2025

This reverts the fix for ticket-27138 I believe that this is still valid as the docs still say that localtime does not to accept naive datetimes (https://docs.djangoproject.com/en/5.2/ref/utils/#django.utils.timezone.localtime) So I'm not sure we should remove this 🤔

Interesting! then what does # Emulate the behavior of astimezone() on Python < 3.6. meant?

@sarahboyce
Copy link
Contributor
sarahboyce commented Apr 30, 2025 •
edited
Loading

I think it means "Maintain the behavior of astimezone() on Python < 3.6", that this logic wasn't required for Python < 3.6 but is required for Python >= 3.6

I'm open to having an update to the comment?

@auvipy
Copy link
Contributor Author
auvipy commented Apr 30, 2025

So I guess just updating the comment should be useful here? I will update the comment and revert the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
no ticket Based on PR title, no linked Trac ticket
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@auvipy @sarahboyce
0