Re: [PATCH v2 1/4] Makefile: extract script to lint missing/extraneous manpages
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 7, 2024, 16:51 UTC
- Message-ID
- <xmqqsexoelw5.fsf@gitster.g>
- In-Reply-To
- <489a6eaf2d328044307f38ff23dd35ecc5a4c515.1717660597.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> The "check-docs" target of our top-level Makefile fulfills two different > roles. For one it runs the "lint-docs" target of the "Documentation/" > Makefile. And second it performs some checks of whether there are any > manpages that are missing or extraneous via some inline scripts.
Yes.
> The second set of checks feels quite misplaced in the top-level Makefile > as it would fit in much better with our "lint-docs" target. Back when > the checks were introduced in 8c989ec528 (Makefile: $(MAKE) check-docs, > 2006-04-13), that target did not yet exist though.
Correct.
Show 5 quoted lines
> Furthermore, the script makes use of several Makefile variables which > are defined in the top-level Makefile, which makes it hard to access > their contents from elsewhere. There is a trick though that we already > use in "check-builtins.sh" to gain access: we can create an ad-hoc > Makefile that has an extra target to print those variables.
Yes.
Show 5 quoted lines
> Pull out the script into a separate "lint-manpages.sh" script by using > that trick. Wire up that script via the "lint-docs" target. For one, > normal shell scripts are way easier to reason about than those which are > embedded in a Makefile. Second, it allows one to easily execute the > script standalone without any of the other checks.
Nicely done. We might want to stop doing "make print-man1" in the lint script, remove "print-man1" target from Documentation/Makefile, and instead extract MAN1_TXT using the same "it is unlikely to have an output line enclosing something in between XXX and YYY" trick, but it may not be worth it.
Show 13 quoted lines
> +check_missing_docs () {
> +...
> + git-merge-octopus) continue;;
> + git-merge-ours) continue;;
> + git-merge-recursive) continue;;
> + git-merge-resolve) continue;;
> + git-merge-subtree) continue;;
> + git-fsck-objects) continue;;
> + git-init-db) continue;;
> + git-remote-*) continue;;
> + git-stage) continue;;
> + git-legacy-*) continue;;
> + git-?*--?* ) continue ;;This looks easier to maintain than the original by listing one pattern per line. Again, nicely done.