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

docs: beef up the ProjectBuilder's method docstrings #178

Merged
merged 1 commit into from
Nov 1, 2020

Conversation

layday
Copy link
Member

@layday layday commented Nov 1, 2020

No description provided.

Copy link
Contributor

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

@@ -169,6 +169,9 @@ def __init__(self, srcdir, config_settings=None, python_executable=sys.executabl

@property
def python_executable(self): # type: () -> Union[bytes, Text]
"""
The Python executable used by :mod:`pep517` to invoke the backend.
Copy link
Member

Choose a reason for hiding this comment

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

Using pep517 is an implementation detail, please remove the reference.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I considered he made the reference here as pep 517 the API, not the package. And in that sense the sentence is right and does not exposes implementation details.

Copy link
Member Author

Choose a reason for hiding this comment

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

It was meant for the package. I'm not sure what the 'executable used by the PEP 517 API' would mean in this context.

Copy link
Contributor

@gaborbernat gaborbernat Nov 1, 2020

Choose a reason for hiding this comment

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

Well, the python executable hosting the build backend (as opposed to the build frontend - current process), https://www.python.org/dev/peps/pep-0517/#build-backend-interface, what else could it?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, :mod: references a module.

Copy link
Member Author

Choose a reason for hiding this comment

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

@gaborbernat I mean that it'd have been confusing to introduce another concept (the 'API') in addition to the frontend and backend - it'd have been somewhat of a tautology. If we wanted to throw PEP 517 in there, I'd have rephrased it as:

The Python executable used to invoke the PEP 517 backend.

But I think it goes without saying that the backend conforms to PEP 517 - build is a PEP 517 frontend.

@FFY00 FFY00 merged commit b3d3c69 into pypa:master Nov 1, 2020
@layday layday deleted the beef-up-docstrings branch March 1, 2024 18:43
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.

3 participants