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

gh-106259: add minimal help target to Makefile #106260

Merged
merged 17 commits into from
Mar 7, 2024
Merged

Conversation

smontanaro
Copy link
Contributor

@smontanaro smontanaro commented Jun 30, 2023

Partially adding a help target, but partially just getting familiar @CAM-Gerlach's git new alias.

Makefile.pre.in Outdated Show resolved Hide resolved
@vstinner
Copy link
Member

vstinner commented Jul 2, 2023

What about putting the documentation there instead? https://docs.python.org/dev/using/configure.html#python-build-system

@smontanaro
Copy link
Contributor Author

My intent was to put more elaborate target documentation somewhere. Haven't started writing anything yet. Thanks for the suggested destination.

@itamaro itamaro added the build The build process and cross-build label Jul 10, 2023
@smontanaro
Copy link
Contributor Author

What about putting the documentation there instead? https://docs.python.org/dev/using/configure.html#python-build-system

I will look into that as well. Having a "make help" command you can execute when you just need a quick reminder is often helpful. And, it's always right at your fingertips.

@merwok merwok changed the title gh-106259: add minimal help target gh-106259: add minimal help target to Makefile Feb 12, 2024
@smontanaro
Copy link
Contributor Author

I made a quick, crude pass at expanding the makefile targets in Doc/using/configure.rst. Some descriptions are essentially unchanged. Other bits I actually expanded on in a hopefully useful way. This still needs more work, but I'm done typing for today, so I'll leave it for now. Feel free to suggest further edits.

Doc/using/configure.rst Outdated Show resolved Hide resolved
Doc/using/configure.rst Outdated Show resolved Hide resolved
Doc/using/configure.rst Outdated Show resolved Hide resolved
Makefile.pre.in Outdated Show resolved Hide resolved
Makefile.pre.in Outdated Show resolved Hide resolved
Makefile.pre.in Outdated Show resolved Hide resolved
Makefile.pre.in Outdated Show resolved Hide resolved
Doc/using/configure.rst Outdated Show resolved Hide resolved
smontanaro and others added 2 commits February 18, 2024 11:24
Co-authored-by: Victor Stinner <[email protected]>
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

Overall, the change LGTM. I just have some minor remarks.

cc @erlend-aasland @corona10: Do you want to review these changes?

Makefile.pre.in Outdated Show resolved Hide resolved
@smontanaro
Copy link
Contributor Author

Any further comments on this @vstinner or @erlend-aasland, or can we push this to completion?

@erlend-aasland
Copy link
Contributor

I'll let @hugovk, @vstinner and @merwok decide and land, since they've been following up this PR :)

@erlend-aasland erlend-aasland removed their request for review February 24, 2024 20:23
Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Looks good! Some small bits and pieces:

Makefile.pre.in Outdated Show resolved Hide resolved
Doc/using/configure.rst Outdated Show resolved Hide resolved
Doc/using/configure.rst Outdated Show resolved Hide resolved
Doc/using/configure.rst Outdated Show resolved Hide resolved
Doc/using/configure.rst Outdated Show resolved Hide resolved
Doc/using/configure.rst Outdated Show resolved Hide resolved
Co-authored-by: Hugo van Kemenade <[email protected]>
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

I would prefer to sort targets in make list-targets but it's a minor issue.

@smontanaro
Copy link
Contributor Author

Thanks. Can someone merge?

@vstinner vstinner merged commit d9ccde2 into python:main Mar 7, 2024
33 checks passed
@vstinner
Copy link
Member

vstinner commented Mar 7, 2024

Merged, thanks @smontanaro!

adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
Co-authored-by: Erlend E. Aasland <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
Co-authored-by: Erlend E. Aasland <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Victor Stinner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build The build process and cross-build skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants