davmac314 / dinit

Service monitoring / "init" system
Apache License 2.0
582 stars 45 forks source link

doc/manpages: New "lint" target to check generated man-pages #331

Closed mobin-2008 closed 3 months ago

mobin-2008 commented 4 months ago

This new script will check every generated man-page to find anything wrong. See https://github.com/davmac314/dinit/pull/330 for an example.

There will be a workflow for checking them in CI.

Signed-off-by: Mobin Aydinfar <mobin@mobintestserver.ir>

mobin-2008 commented 4 months ago

@davmac314 Is it useful for man-pages? I think yes.

Currently it's marked as draft because we need to adjust warnings that we want and it's missing a CI workflow. It's an example of script's output (except "date" warning):

$ make -C doc/manpages/ lint
gmake: Entering directory '/home/mobin/Documents/git/dinit/doc/manpages'
m4 -DVERSION=0.18.1pre -DMONTH=January -DYEAR=2024 -DDEFAULT_AUTO_RESTART=true\
   -DDEFAULT_START_TIMEOUT=60\
   -DDEFAULT_STOP_TIMEOUT=10 dinit-service.5.m4 > dinit-service.5
./mandoc-lint.sh
mandoc: dinit.8:237:2: WARNING: skipping paragraph macro: PP after SH
mandoc: dinit.8:286:2: WARNING: skipping paragraph macro: PP after SH
mandoc: dinitctl.8:252:1: WARNING: invalid escape sequence: \f[C]
mandoc: dinitctl.8:253:1: WARNING: invalid escape sequence: \f[C]
mandoc: dinitctl.8:254:1: WARNING: invalid escape sequence: \f[C]
mandoc: dinitctl.8:255:1: WARNING: invalid escape sequence: \f[C]
mandoc: dinitctl.8:256:1: WARNING: invalid escape sequence: \f[C]
mandoc: dinitctl.8:257:1: WARNING: invalid escape sequence: \f[C]
mandoc: dinit-monitor.8:43:42: WARNING: invalid escape sequence: \br\fB\-\-test\fR
mandoc: dinit-service.5:638:2: ERROR: skipping end of block that is not open: RE
mandoc: dinit-service.5:638:2: WARNING: skipping paragraph macro: br at the end of SS
mandoc: dinit-service.5:717:2: WARNING: skipping paragraph macro: PP after SH
There is some error/warning around man-pages, Exiting with 1...
gmake: *** [Makefile:27: lint] Error 1
gmake: Leaving directory '/home/mobin/Documents/git/dinit/doc/manpages'
mobin-2008 commented 4 months ago

CI workflow has been added. Failure is because of current warnings from mandoc.

davmac314 commented 4 months ago

I'm not convinced these warnings are accurate enough generally to make this a gate requirement for CI. I see both incorrect warnings:

mandoc: dinit.8:237:2: WARNING: skipping paragraph macro: PP after SH

(It's an LP after the SH, not a PP)

and warnings about sequences that groff understands but that mandoc doesn't:

mandoc: dinitctl.8:252:1: WARNING: invalid escape sequence: \f[C]

I haven't evaluated the rest of the warnings. As the person who instigated this, you should do that yourself. Figure out what the warnings mean, whether they are correct, what is required to fix them and whether it is worth doing so. There is at least one thing actually worth fixing - so it would be good if you could raise a PR to fix that and any other genuine issues - but unless you can provide a better idea of how actually useful this is, I don't want it to be part of CI.

davmac314 commented 3 months ago

I have now fixed some of the causes of the warnings I could see were genuine.

However as I mentioned, I don't want to incorporate a check into CI that resolves on a fairly hacky manual filtering of warning messages and no clarity as to how severe or genuine the warnings it may generate are.

@mobin-2008 can we close this?

mobin-2008 commented 3 months ago

I have now fixed some of the causes of the warnings I could see were genuine.

However as I mentioned, I don't want to incorporate a check into CI that resolves on a fairly hacky manual filtering of warning messages and no clarity as to how severe or genuine the warnings it may generate are.

@mobin-2008 can we close this?

I think it will be good as a manual target when we change the man-pages. I will think about it when I closed some other works of mine.