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

feat: Support Python 3.12 #942

Merged
merged 3 commits into from
Nov 18, 2023
Merged

Conversation

edgarrmondragon
Copy link
Contributor

@edgarrmondragon edgarrmondragon commented Oct 18, 2023

What do these changes do?

Communicates official support for Python 3.12

Are there changes in behavior for the user?

No

Related issue number

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> (e.g. 588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the PR
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: Fix issue with non-ascii contents in doctest text files.

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Oct 18, 2023
@edgarrmondragon edgarrmondragon marked this pull request as ready for review October 18, 2023 04:38
@codecov

This comment was marked as outdated.

@amotl
Copy link

amotl commented Nov 12, 2023

Dear @asvetlov and @webknjaz,

thanks a stack for conceiving and maintaining the excellent yarl package. We have been tripped by missing support for Python 3.12 at daq-tools/influxio#57.

May we humbly ask if you have the capacity to bring in this patch, or, alternatively, if you would accept any kind of support on this matter?

Keep up the spirit, and with kind regards,
Andreas.

@edgarrmondragon
Copy link
Contributor Author

The linter checks seem to be failing upstream in master too 😕

@webknjaz
Copy link
Member

Yep, I'll fix that. Could you use the rebase button next time?

@webknjaz
Copy link
Member

@amotl this PR only modifies some metadata. The runtime support is already present on master.

@webknjaz webknjaz force-pushed the python3.12 branch 2 times, most recently from 113b75a to 7fffb3c Compare November 18, 2023 00:00
Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

Additionally, I'd add a separate change note 942.contrib.rst to notify the contributors about requiring Python 3.12 CI jobs to pass in pull requests.

@@ -0,0 +1 @@
Official support for Python 3.12.
Copy link
Member

Choose a reason for hiding this comment

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

We write (most of the) individual change note fragments in the past tense, because they're targeted at the end-users and don't necessarily contain information about internal implementation details or decision.
Changelog is supposed to describe what changed compared to the previous version.

Individual fragments should have the same style. When looking at individual PRs on GitHub, it's unobvious and is hard to judge whether it “fits”.

However, we have RTD PR builds enabled, so it's possible to preview at https://yarl--942.org.readthedocs.build/en/942/changes.html whether a fragment contributes to the changelog being cohesive.

I think, this phrase can be rephrased to describe more accurately what the PR does, with the end-users being the target audience in mind.

Suggested change
Official support for Python 3.12.
Declared Python 3.12 supported officially in the distribution package metadata
-- by :user:`edgarrmondragon`.

Copy link
Member

Choose a reason for hiding this comment

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

With the below note in mind, I'd change this fragment type from misc to packaging. I recently added more clear categories for the Towncrier fragments that should reflect the changelog sections better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, done in 810c06e

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, PSF Chronographer / Timeline protection is complaining now:

No files matching re.compile('news/(?P<issue_number>[^\\./]+)\.(?P<fragment_type>feature|bugfix|doc|removal|misc)(\.\d+)?(\.[^\\./]+)*$') pattern added

https://github.com/aio-libs/yarl/pull/942/checks?check_run_id=18803866754

Copy link
Member

Choose a reason for hiding this comment

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

Ouch.. Looks like I need to teach the bot about an extra config file. Totally not your fault. I'll force-merge if need be.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Exactly. When I first wrote that code, I didn't know that an alternative config path fallback exists. Though, it's no big deal. I'll correct it and redeploy later.

@webknjaz
Copy link
Member

So it looks like that contrib file is still needed..

@webknjaz webknjaz merged commit 2f5e6b1 into aio-libs:master Nov 18, 2023
39 of 40 checks passed
@edgarrmondragon edgarrmondragon deleted the python3.12 branch November 18, 2023 15:04
@amotl
Copy link

amotl commented Nov 23, 2023

Hi again,

thank you so much for concluding this patch and running a release. This downstream patch demonstrates it works well.

Keep up the spirit, and with kind regards,
Andreas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided There is a change note present in this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants