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

doc: add documentation for using extension modules #9864

Merged
merged 2 commits into from
Nov 26, 2024

Conversation

abn
Copy link
Member

@abn abn commented Nov 21, 2024

No description provided.

@abn abn added the area/docs Documentation issues/improvements label Nov 21, 2024
@abn abn marked this pull request as ready for review November 21, 2024 23:37
@abn abn added the impact/docs Contains or requires documentation changes label Nov 21, 2024
Copy link

github-actions bot commented Nov 21, 2024

Deploy preview for website ready!

✅ Preview
https://website-7ko3zlrr8-python-poetry.vercel.app

Built with commit 80bc1fb.
This pull request is being automatically deployed with vercel-action

@abn abn force-pushed the docs/extension-build-script branch 5 times, most recently from 6864718 to 7525d7e Compare November 22, 2024 00:16
@abn abn mentioned this pull request Nov 22, 2024
2 tasks
@abn abn force-pushed the docs/extension-build-script branch 2 times, most recently from 8f8d00b to 7f9b79a Compare November 22, 2024 00:35
@smith120bh
Copy link

@abn : I spotted this PR from your post on the issue, which I've been following. This looks great! Just one thing I noticed - this statement looks mutually contradictory to me? I've personally not tried putting a build script in a subdirectory, but I had thought that it needed to be in the root directory, next to pyproject.toml.
image

@abn
Copy link
Member Author

abn commented Nov 22, 2024

Good catch, I should be clearer there. What I wanted to communicate was that it must in the root or a subdirectory. Where you place the script won't matter as long as it is included in your sdist and referred to correctly within your pyproject file. I will fix it up to be clearer.

@abn abn force-pushed the docs/extension-build-script branch from 7f9b79a to 2a2d10b Compare November 22, 2024 01:07
@abn abn requested a review from a team November 22, 2024 01:20
@abn abn force-pushed the docs/extension-build-script branch 2 times, most recently from 47da77b to ce80f31 Compare November 22, 2024 01:31
Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

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

Some first feedback. (I have not looked at the examples in details yet.)

You did not mention generate-setup-file. (Personally, I have no idea if/when you need this setting.) Should it be mentioned?

docs/building-extension-modules.md Outdated Show resolved Hide resolved
docs/building-extension-modules.md Outdated Show resolved Hide resolved
docs/building-extension-modules.md Show resolved Hide resolved
docs/building-extension-modules.md Outdated Show resolved Hide resolved
docs/building-extension-modules.md Outdated Show resolved Hide resolved
docs/building-extension-modules.md Outdated Show resolved Hide resolved
@Secrus
Copy link
Member

Secrus commented Nov 22, 2024

You did not mention generate-setup-file. (Personally, I have no idea if/when you need this setting.) Should it be mentioned?

That's a good thing. I'd personally be glad to see that option gone, since it ties us to setuptools. It was useful when PEP 517 was still new and not all tools supported build backends different than setuptools, but nowadays it has limited use.

@abn
Copy link
Member Author

abn commented Nov 22, 2024

Yes. The non-mention of that option is intentional. Today, it defaults to false, and we may remove it in the future. Especially as the setup file generation is full of holes.

@abn abn force-pushed the docs/extension-build-script branch 4 times, most recently from 76b9dd1 to 4932b00 Compare November 22, 2024 16:56
@abn abn requested a review from radoering November 22, 2024 17:07
Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

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

I have only skimmed over the examples and have not tried them out, but since there is a warning at the beginning of the section, I think it is ok to publish it and wait for feedback.

Btw, do you want to squash the commits? They have the same message and I do not see that keeping them separate adds any value.

@abn
Copy link
Member Author

abn commented Nov 26, 2024

Squash is fine. I think I forgot to squash the last push.

@abn abn force-pushed the docs/extension-build-script branch from 678490d to 80bc1fb Compare November 26, 2024 20:29
@abn abn merged commit b7cd0b0 into python-poetry:main Nov 26, 2024
74 checks passed
@abn abn deleted the docs/extension-build-script branch November 26, 2024 20:31
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/docs Documentation issues/improvements impact/docs Contains or requires documentation changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants