FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Mark all targets in `Doc/Makefile` as `PHONY` by sobolevn · Pull Request #98189 · python/cpython · GitHub

/ cpython Public

Mark all targets in Doc/Makefile as PHONY - #98189

Merged
JulienPalard merged 1 commit into
python:mainfrom
sobolevn:patch-12
Oct 12, 2022
ConversationCommits (1)ChecksFiles changed
Merged

Mark all targets in Doc/Makefile as PHONY#98189
JulienPalard merged 1 commit into
python:mainfrom
sobolevn:patch-12

Conversation

sobolevn commented Oct 11, 2022

Copy link
Copy Markdown
Member

Some of them were missing fro PHONY list. Some of them were in correct order.
Now order of PHONY and order of definitions match.

Docs on PHONY: https://www.gnu.org/software/make/manual/html_node/Phony-Targets.html

sobolevn requested a review from JulienPalard October 11, 2022 19:33
JulienPalard merged commit 4414586 into python:main Oct 12, 2022

JulienPalard commented Oct 12, 2022

Copy link
Copy Markdown
Member

Thanks! We should keep a better eye on this one ☺

zware commented Oct 12, 2022

Copy link
Copy Markdown
Member

I've found it easier to keep track of .PHONY targets by doing something like:

.PHONY: help
help:
	@echo ...

.PHONY: build
build:
	...

.PHONY: html
html: BUILDER = html
html: build
	@echo ...

It's more verbose, and I am not certain that all makes will handle it correctly, but I've found this style much easier to keep up to date. Perhaps a consideration for the future?

JulienPalard commented Oct 12, 2022

Copy link
Copy Markdown
Member

Perhaps, yes.

Having the .PHONY near the rule is what I'm acustomed to do too, but I've seen many makefiles with all PHONY's on the same line like here.

I never heard about compatibility issues (but I'm only using GNU make so ... I'm not a reference).

sobolevn commented Oct 12, 2022

Copy link
Copy Markdown
Member Author

I am also using .PHONY on target level, example: https://github.com/sobolevn/git-secret/blob/master/Makefile#L12

I never got any complaints :)

sobolevn commented Oct 12, 2022

Copy link
Copy Markdown
Member Author

Thanks for the quick merge! 👍

hugovk mentioned this pull request Oct 14, 2022
Closed

hugovk commented Oct 14, 2022

Copy link
Copy Markdown
Member

I've found it easier to keep track of .PHONY targets by doing something like:

.PHONY: help
help:
	@echo ...

.PHONY: build
build:
	...

.PHONY: html
html: BUILDER = html
html: build
	@echo ...

It's more verbose, and I am not certain that all makes will handle it correctly, but I've found this style much easier to keep up to date. Perhaps a consideration for the future?

Agreed, I expect it will happen again because it's normal to copy/paste a target and forget (or not know) to update the .PHONY line right at the top.

I was making some other fixes to the Makefile, so included this too: #98266

carljm added a commit to carljm/cpython that referenced this pull request Oct 14, 2022
Merge branch 'main' into typewatch
6e53514 
* main: (38 commits)
  pythongh-98251: Allow venv to pass along PYTHON* variables to pip and ensurepip when they do not impact path resolution (pythonGH-98259)
  Bpo-41246: IOCP Proactor avoid callback code duplication (python#21399)
  bpo-46364: Use sockets for stdin of asyncio only on AIX (python#30596)
  pythongh-98178: syslog() is not thread-safe on macOS (python#98213)
  Mark all targets in `Doc/Makefile` as `PHONY` (pythonGH-98189)
  pythongh-97982: Factorize PyUnicode_Count() and unicode_count() code (python#98025)
  pythongh-96265: Formatting changes for faq/general (python#98129)
  tutorial: remove "with single quotes" (python#98204)
  pythongh-97669: Remove Tools/scripts/startuptime.py (python#98214)
  signalmodule.c uses _PyErr_WriteUnraisableMsg() (python#98217)
  pythongh-97669: Fix test_tools reference leak (python#98216)
  pythongh-97669: Create Tools/patchcheck/ directory (python#98186)
  pythongh-65046: Link to logging cookbook from asyncio docs (python#98207)
  Formatting fixes in contextlib docs (python#98111)
  pythongh-95276: Add callable entry to the glossary (python#95738)
  pythongh-96130: Rephrase use of "typecheck" verb for clarity (python#98144)
  Fix some incorrect indentation around the main switch (python#98177)
  pythongh-98172: Fix formatting in `except*` docs (python#98173)
  pythongh-97982: Remove asciilib_count() (python#98164)
  pythongh-95756: Free and NULL-out code caches when needed (pythonGH-98181)
  ...
carljm added a commit to carljm/cpython that referenced this pull request Oct 14, 2022
Merge branch 'main' into cached_property
0bb0c69 
* main: (37 commits)
  pythongh-98251: Allow venv to pass along PYTHON* variables to pip and ensurepip when they do not impact path resolution (pythonGH-98259)
  Bpo-41246: IOCP Proactor avoid callback code duplication (python#21399)
  bpo-46364: Use sockets for stdin of asyncio only on AIX (python#30596)
  pythongh-98178: syslog() is not thread-safe on macOS (python#98213)
  Mark all targets in `Doc/Makefile` as `PHONY` (pythonGH-98189)
  pythongh-97982: Factorize PyUnicode_Count() and unicode_count() code (python#98025)
  pythongh-96265: Formatting changes for faq/general (python#98129)
  tutorial: remove "with single quotes" (python#98204)
  pythongh-97669: Remove Tools/scripts/startuptime.py (python#98214)
  signalmodule.c uses _PyErr_WriteUnraisableMsg() (python#98217)
  pythongh-97669: Fix test_tools reference leak (python#98216)
  pythongh-97669: Create Tools/patchcheck/ directory (python#98186)
  pythongh-65046: Link to logging cookbook from asyncio docs (python#98207)
  Formatting fixes in contextlib docs (python#98111)
  pythongh-95276: Add callable entry to the glossary (python#95738)
  pythongh-96130: Rephrase use of "typecheck" verb for clarity (python#98144)
  Fix some incorrect indentation around the main switch (python#98177)
  pythongh-98172: Fix formatting in `except*` docs (python#98173)
  pythongh-97982: Remove asciilib_count() (python#98164)
  pythongh-95756: Free and NULL-out code caches when needed (pythonGH-98181)
  ...
hugovk mentioned this pull request Oct 21, 2022
Merged
hugovk mentioned this pull request Nov 11, 2022
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

JulienPalard Awaiting requested review from JulienPalard

Assignees

No one assigned

Labels

skip issue skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL