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

A unified pyproject.toml specification #1396

Merged
merged 9 commits into from
Dec 4, 2023

Conversation

jeanas
Copy link
Contributor

@jeanas jeanas commented Nov 14, 2023

There are currently two specifications describing pyproject.toml, Declaring build system dependencies and Declaring project metadata. This split corresponds to history: first PEP 518 came, then PEP 621.

The two specifications are closely related (they define tables of the same file), and it's also slightly confusing to have the [tool] table defined in the "Declaring build system dependencies" spec. This PR proposes a unified spec.

Each commit is atomic so you can check that I didn't sneak in spec changes :)


📚 Documentation preview 📚: https://python-packaging-user-guide--1396.org.readthedocs.build/en/1396/

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.

Nice merge of the 2 specs. I would suggest putting the note at the top to identify the history of the 2 PEPs.

source/specifications/pyproject-toml.rst Show resolved Hide resolved
source/specifications/pyproject-toml.rst Outdated Show resolved Hide resolved
Co-authored-by: Carol Willing <[email protected]>
@jeanas
Copy link
Contributor Author

jeanas commented Nov 17, 2023

(Fixing the merge conflict with #1395.)

@jeanas
Copy link
Contributor Author

jeanas commented Nov 22, 2023

Given that 2 people approved, is this ready to go? I'm asking because there is now #1414 building on top.

@jeanas
Copy link
Contributor Author

jeanas commented Dec 3, 2023

Gentle ping? @willingc?

@pradyunsg pradyunsg added this pull request to the merge queue Dec 4, 2023
@pradyunsg
Copy link
Member

IIUC, this is approved already and reviewed by multiple people -- trusting the group here and marking this into the queue without doing a review myself.

Merged via the queue into pypa:main with commit af82fef Dec 4, 2023
5 checks passed
@jeanas jeanas deleted the unified-pyroject-spec branch December 4, 2023 19:35
@jeanas
Copy link
Contributor Author

jeanas commented Dec 4, 2023

Thank you @willingc, @sinoroc and @pradyunsg!

@jeanas
Copy link
Contributor Author

jeanas commented Dec 4, 2023

@pradyunsg However, we need redirects in the RTD config to keep the old links working. (Sorry, I was sure I had mentioned this in the PR description.) Namely, from /specifications/declaring-build-dependencies/ and /specifications/declaring-project-metadata/ to /specifications/pyproject-toml/.

Also, I'm puzzled because I thought the GitHub merge queue was supposed to check the merged result, but CI passed on this PR even though it conflicts with some of @sinoroc's work on main that adds references to labels that I renamed 😠 I'm going to prepare a PR to fix that right now and then investigate why the CI didn't work as I thought...

@hugovk
Copy link
Contributor

hugovk commented Dec 5, 2023

See also #1434, would it be okay to keep the declaring-project-metadata reference?

We're relying on it to build PEP 621 in the PEPs repo via intersphinx. And/or we can fix PEP 621 to use the new ref.

But it might be worth keeping the old refs to avoid breaking intersphinx for other people/avoid linkrot. (We usually do this in https://github.com/python/devguide.)

@jeanas
Copy link
Contributor Author

jeanas commented Dec 5, 2023

Some updates here:

@pradyunsg However, we need redirects in the RTD config to keep the old links working. (Sorry, I was sure I had mentioned this in the PR description.) Namely, from /specifications/declaring-build-dependencies/ and /specifications/declaring-project-metadata/ to /specifications/pyproject-toml/.

Opened #1437.

Also, I'm puzzled because I thought the GitHub merge queue was supposed to check the merged result, but CI passed on this PR even though it conflicts with some of @sinoroc's work on main that adds references to labels that I renamed 😠 I'm going to prepare a PR to fix that right now and then investigate why the CI didn't work as I thought...

I've figured that out­ — it's pretty nasty. See #1438.

See also #1434, would it be okay to keep the declaring-project-metadata reference?

This is being discussed in #1434.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants