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.

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

gh-108219: Add credits to free-threading project #123802

Merged
merged 8 commits into from
Sep 8, 2024
Merged

gh-108219: Add credits to free-threading project #123802

merged 8 commits into from
Sep 8, 2024

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Sep 7, 2024 •
edited by github-actions bot
Loading

@bedevere-app bedevere-app bot added the docs Documentation in the Doc dir label Sep 7, 2024
@corona10 corona10 added needs backport to 3.13 bugs and security fixes and removed docs Documentation in the Doc dir labels Sep 7, 2024
willingc
willingc reviewed Sep 7, 2024
View reviewed changes
Doc/whatsnew/3.13.rst Outdated
@@ -354,6 +354,10 @@ disabled with the :envvar:`PYTHON_GIL` environment variable or the
pip 24.1 or newer is required to install packages with C extensions in the
free-threaded build.

This project has been contributed by many people and the community,
including Brett Simmers, Dino Viehland, Donghee Na, Ken Jin, Matt Page, and Sam Gross.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this include @iritkatriel and a few others from Microsoft too?

Copy link
Member

Choose a reason for hiding this comment

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

No, not me.

Copy link
Member Author

@corona10 corona10 Sep 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Please let me know if I miss someone at Microsoft team.
cc @Fidget-Spinner

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure about this, I think there are too many contributors to try listing them exhaustively. Maybe go with something like "Sam Gross and many others"?

Copy link
Member Author

@corona10 corona10 Sep 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Disclaimer: As the person listed in the description, I am biased, so I have no opinions about it.

But listing people as possible can give motivation to work projects not only for this but also other projects. For the faster-cpython project, they listed as many people as possible, so it was a great honor for me, and it motivated me to participate in other projects.
(And It can also be a good reference for persuading a company to let people work part-time on the CPython project.)

"Sam Gross and many others"?

I think this is the minimum option that should be used if there is controversy regarding this.

Copy link
Member Author

Choose a reason for hiding this comment

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

And I also think there should be more people on the list. Please let me know or have the person update the description with more details.
(I just picked people with free-threading labels)

Copy link
Member

@Fidget-Spinner Fidget-Spinner Sep 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I can't speak for the MS team because I'm also not employed by them :) but IMO/IIUC the MS team is focused on single-threaded JIT performance mostly. So they werent much involved in the 3.13 free threaded effort, apart from discussions and design help we received from them, which I'm thankful for.

AA-Turner
AA-Turner reviewed Sep 8, 2024
View reviewed changes
Doc/whatsnew/3.13.rst Outdated
Comment on lines 357 to 360
This project has been contributed by many people and the community,
including Brett Simmers, Dino Viehland, Donghee Na, Itamar Oren, Ken Jin,
Matt Page, and Sam Gross.
Meta has supported in the form of many engineer-years to this project.
Copy link
Member

@AA-Turner AA-Turner Sep 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Wording suggestions (on mobile so I can't easily check line wrapping)

Suggested change
This project has been contributed by many people and the community,
including Brett Simmers, Dino Viehland, Donghee Na, Itamar Oren, Ken Jin,
Matt Page, and Sam Gross.
Meta has supported in the form of many engineer-years to this project.
This work is thanks to many people and organisations, including the large community of contributors
to Python and third party projects to test and enable free-threading support.
This includes, but is not limited to:
Sam Gross, Ken Jin, Donghee Na, Itamar Oren, Matt Page, Brett Simmers, and Dino Viehland.
Many of these contributors are employed by Meta Inc,
which has provided significant manpower to support this project.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, it's more better, and @itamaro is going to add more detail by next week :)

@corona10 @AA-Turner
Update Doc/whatsnew/3.13.rst
9929867 
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
hugovk
hugovk reviewed Sep 8, 2024
View reviewed changes
Doc/whatsnew/3.13.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.13.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.13.rst Outdated Show resolved Hide resolved
corona10 and others added 2 commits September 8, 2024 19:03
@corona10 @hugovk
Apply suggestions from code review
d900780 
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@corona10
Use a gender-neutral term
33be59e
itamaro
itamaro reviewed Sep 8, 2024
View reviewed changes
Doc/whatsnew/3.13.rst Outdated Show resolved Hide resolved
@corona10 @itamaro
Update Doc/whatsnew/3.13.rst
79ec285 
Co-authored-by: Itamar Oren <itamarost@gmail.com>
willingc
willingc approved these changes Sep 8, 2024
View reviewed changes
Copy link
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

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

@corona10 Thanks for making the effort to be inclusive with the credits after my initial comment. Looks good to me now.

@AA-Turner AA-Turner merged commit aa3f11f into python:main Sep 8, 2024
23 checks passed
@miss-islington-app
Copy link

Thanks @corona10 for the PR, and @AA-Turner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 8, 2024
@corona10 @AA-Turner
@hugovk @itamaro @miss-islington
pythongh-108219: Add credits to the free-threading entry in What's New (
f368d39 
pythonGH-123802)

(cherry picked from commit aa3f11f)

Co-authored-by: Donghee Na <donghee.na@python.org>
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Itamar Oren <itamarost@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Sep 8, 2024

GH-123847 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Sep 8, 2024
@corona10 corona10 deleted the gh-108219-authors branch September 9, 2024 01:07
Yhg1s pushed a commit that referenced this pull request Sep 24, 2024
@miss-islington @corona10
@AA-Turner @hugovk @itamaro
[3.13] gh-108219: Add credits to the free-threading entry in What's N…
87f51a2 
…ew (GH-123802) (#123847)

gh-108219: Add credits to the free-threading entry in What's New (GH-123802)
(cherry picked from commit aa3f11f)

Co-authored-by: Donghee Na <donghee.na@python.org>
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Itamar Oren <itamarost@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iritkatriel iritkatriel iritkatriel left review comments

@Fidget-Spinner Fidget-Spinner Fidget-Spinner left review comments

@itamaro itamaro itamaro approved these changes

@willingc willingc willingc approved these changes

@AA-Turner AA-Turner AA-Turner approved these changes

@colesbury colesbury Awaiting requested review from colesbury

@Yhg1s Yhg1s Awaiting requested review from Yhg1s

@hugovk hugovk Awaiting requested review from hugovk

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@corona10 @itamaro @iritkatriel @hugovk @willingc @AA-Turner @Fidget-Spinner