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

doc: remove unnecessary macros #5801

Merged
merged 1 commit into from
Jul 17, 2023
Merged

doc: remove unnecessary macros #5801

merged 1 commit into from
Jul 17, 2023

Conversation

osalyk
Copy link
Contributor

@osalyk osalyk commented Jul 14, 2023

This change is Reviewable

@osalyk osalyk added documentation doc/ and other Markdown files sprint goal This pull request is part of the ongoing sprint labels Jul 14, 2023
Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewed 19 of 19 files at r1, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @osalyk)


doc/macros.man line 9 at r1 (raw file):

#
# This solution allows the maintenance of Windows, Linux and FreeBSD
# documentation in the same file.

Suggestion:

# These macros are defined for the m4 preprocessor and allow easier maintenance
# of repetitive but cumbersome snippets.

doc/macros.man line 18 at r1 (raw file):

#		Inserts suggested pathnames for LD_LIBRARY_PATH.
#	_MP(man_page_name, section):
#		Include the man page section number if not building for WEB.

You could just move these docstrings next to the respective defines. Just a personal preference.


doc/macros.man line 23 at r1 (raw file):

changecom()
define(_DEBUGLIBPATH, **/usr/lib/pmdk_debug**)
define(_LDLIBPATH, =q==q==q=**/usr/lib/pmdk_debug** or **/usr/lib64/pmdk_debug**, as appropriate=e==e==e=)

I believe a single quote is enough.

Suggestion:

define(_LDLIBPATH, =q=**/usr/lib/pmdk_debug** or **/usr/lib64/pmdk_debug**, as appropriate=e=)

doc/macros.man line 24 at r1 (raw file):

define(_DEBUGLIBPATH, **/usr/lib/pmdk_debug**)
define(_LDLIBPATH, =q==q==q=**/usr/lib/pmdk_debug** or **/usr/lib64/pmdk_debug**, as appropriate=e==e==e=)
define(_MP, ifdef(=q=WEB=e=,$1,$1($2)))

I could not find a single instance where this macro is still being used. As far as I can tell, the only place where it is still mentioned is here.

But to me, it seems a leftover and this line should be replaced with something like this:

title\=\`sed \-n 's/^title:\\ "\\(\[A-Za-z0-9\_-\]\*\\) | .\*"/\\1/p' $filename\`

Where sections AFAICT are completely removed from the source markdown files. 🤦‍♂️

As of now, it is a bug since the generated man files do not have defined titles and sections.

FYI @grom72


doc/libpmem/pmem_is_pmem.3.md line 36 at r1 (raw file):

int pmem_is_pmem(const void *addr, size_t len);
void *pmem_map_file(const qchar *path, size_t len, int flags,

Suggestion:

void *pmem_map_file(const char *path, size_t len, int flags,

doc/libpmem/pmem_is_pmem.3.md line 37 at r1 (raw file):

int pmem_is_pmem(const void *addr, size_t len);
void *pmem_map_file(const qchar *path, size_t len, int flags,
	mode_t mode, size_t *mapped_lenp, int *is_pmemp);

👏


doc/libpmempool/pmempool_sync.3.md line 112 at r1 (raw file):

applied. The effective size of a replica is the sum of sizes of all its part
files decreased by 4096 bytes per each part file. The 4096 bytes of each part
file is utilized for storing internal metadata of the pool part files.=e=)

I might be a little tired at this point but both these paragraphs seem to be Windows-specific since they come before any comma in the _WINUX() call, aren't they?

But are they really Windows-specific, I am not sure to be hones. 🤣


doc/pmempool/pmempool-transform.1.md line 65 at r1 (raw file):

Effective size of a replica is the sum of sizes of all its part files decreased
by 4096 bytes per each part file. The 4096 bytes of each part file is
utilized for storing internal metadata of the pool part files.=e=)

The same here. They seem to be defined as Windows-specific.

Copy link
Contributor Author

@osalyk osalyk left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @grom72 and @janekmi)


doc/macros.man line 9 at r1 (raw file):

#
# This solution allows the maintenance of Windows, Linux and FreeBSD
# documentation in the same file.

Done.


doc/macros.man line 23 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I believe a single quote is enough.

Done.


doc/macros.man line 24 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I could not find a single instance where this macro is still being used. As far as I can tell, the only place where it is still mentioned is here.

But to me, it seems a leftover and this line should be replaced with something like this:

title\=\`sed \-n 's/^title:\\ "\\(\[A-Za-z0-9\_-\]\*\\) | .\*"/\\1/p' $filename\`

Where sections AFAICT are completely removed from the source markdown files. 🤦‍♂️

As of now, it is a bug since the generated man files do not have defined titles and sections.

FYI @grom72

Done.


doc/libpmem/pmem_is_pmem.3.md line 36 at r1 (raw file):

int pmem_is_pmem(const void *addr, size_t len);
void *pmem_map_file(const qchar *path, size_t len, int flags,

Done.


doc/libpmempool/pmempool_sync.3.md line 112 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I might be a little tired at this point but both these paragraphs seem to be Windows-specific since they come before any comma in the _WINUX() call, aren't they?

But are they really Windows-specific, I am not sure to be hones. 🤣

Done.


doc/pmempool/pmempool-transform.1.md line 65 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

The same here. They seem to be defined as Windows-specific.

Done.

Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

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

Reviewed 18 of 19 files at r1, 5 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @janekmi and @osalyk)


doc/macros.man line 23 at r1 (raw file):

Previously, osalyk (Oksana Sałyk) wrote…

Done.

Do we need these macros at all?
What will happen if we abandon macros.man entirely?


doc/macros.man line 15 at r2 (raw file):

changequote(=q=,=e=)
changecom()

Are these used anywhere?

Code quote:

changequote(=q=,=e=)
changecom()

doc/macros.man line 80 at r2 (raw file):

ifdef(=q=FREEBSD=e=,**/usr/local/lib/pmdk_debug**,
=q==q==q=**/usr/lib/pmdk_debug** or **/usr/lib64/pmdk_debug**, as appropriate=e==e==e=)))
define(_MP, ifdef(=q=WEB=e=,$1,$1($2)))

Can we abandon this macro? if we do not abandon the WEB version of manuals?

Code quote:

define(_MP, ifdef(=q=WEB=e=,$1,$1($2)))

doc/libpmem/libpmem.7.md line 43 at r2 (raw file):

const char *pmem_check_version(
	unsigned major_required,
	unsigned minor_required);

Suggestion:

const char *pmem_check_version(unsigned major_required,
	unsigned minor_required);

doc/libpmem/pmem_flush.3.md line 100 at r2 (raw file):

>WARNING:
On Linux, **pmem_msync**() and **msync**(2) have no effect on memory ranges

DO we have any context outside LInux?


doc/libpmem/pmem_is_pmem.3.md line 37 at r2 (raw file):

int pmem_is_pmem(const void *addr, size_t len);
void *pmem_map_file(const char *path, size_t len, int flags,
	mode_t mode, size_t *mapped_lenp, int *is_pmemp);

Suggestion:

void *pmem_map_file(const char *path, size_t len, int flags, mode_t mode,
	size_t *mapped_lenp, int *is_pmemp);

doc/libpmemobj/libpmemobj.7.md line 42 at r1 (raw file):

const char *pmemobj_check_version(
	unsigned major_required,
	unsigned minor_required);

Suggestion:

const char *pmemobj_check_version(unsigned major_required,
	unsigned minor_required);

doc/libpmempool/libpmempool.7.md line 41 at r2 (raw file):

const char *pmempool_check_version(
	unsigned major_required,
	unsigned minor_required);

Suggestion:

const char *pmempool_check_version(unsigned major_required,
	unsigned minor_required);

doc/libpmempool/pmempool_sync.3.md line 34 at r2 (raw file):

int pmempool_sync(const char *poolset_file,
	unsigned flags); (EXPERIMENTAL)

One line, please

Suggestion:

int pmempool_sync(const char *poolset_file, unsigned flags); (EXPERIMENTAL)

utils/md2man.sh line 30 at r2 (raw file):

template=$2
outfile=$3
title=`sed -n 's/^title:\ _MP(*\([A-Za-z0-9_-]*\).*$/\1/p' $filename`

Do we abandon WEB support?

Code quote:

_MP(

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewed 5 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @grom72, @janekmi, and @osalyk)


doc/macros.man line 15 at r2 (raw file):
changequote() is still crucial for the _LDLIBPATH() definition.
changecom():

Calling changecom without any arguments, or with start as the empty string, will effectively disable the commenting mechanism. To restore the original comment start of ‘#’, you must explicitly ask for it.

So, I guess it might help pass comments through the macro processor which otherwise would be missing from the output.

Ref: https://www.gnu.org/software/m4/manual/html_node/Changequote.html
Ref: https://www.gnu.org/software/m4/manual/html_node/Changecom.html


doc/macros.man line 80 at r2 (raw file):

You would have to copy-paste these paths where this "function" is called.
Which is not much I guess.

You would have to copy-paste these paths where this "function" is called.
Which is not much I guess.


doc/libpmem/libpmem.7.md line 43 at r2 (raw file):

const char *pmem_check_version(
	unsigned major_required,
	unsigned minor_required);

I preferred the previous formatting better. Then you could see all arguments next to each other.


doc/libpmemobj/libpmemobj.7.md line 42 at r1 (raw file):

const char *pmemobj_check_version(
	unsigned major_required,
	unsigned minor_required);

.


doc/libpmempool/libpmempool.7.md line 41 at r2 (raw file):

const char *pmempool_check_version(
	unsigned major_required,
	unsigned minor_required);

.


utils/md2man.sh line 30 at r2 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

Do we abandon WEB support?

🙊

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @grom72 and @osalyk)

Copy link
Contributor Author

@osalyk osalyk left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @grom72)


doc/macros.man line 15 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

changequote() is still crucial for the _LDLIBPATH() definition.
changecom():

Calling changecom without any arguments, or with start as the empty string, will effectively disable the commenting mechanism. To restore the original comment start of ‘#’, you must explicitly ask for it.

So, I guess it might help pass comments through the macro processor which otherwise would be missing from the output.

Ref: https://www.gnu.org/software/m4/manual/html_node/Changequote.html
Ref: https://www.gnu.org/software/m4/manual/html_node/Changecom.html

Done.


doc/macros.man line 80 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

You would have to copy-paste these paths where this "function" is called.
Which is not much I guess.

You would have to copy-paste these paths where this "function" is called.
Which is not much I guess.

Done.


doc/libpmem/libpmem.7.md line 43 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I preferred the previous formatting better. Then you could see all arguments next to each other.

Done.


doc/libpmem/pmem_flush.3.md line 100 at r2 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

DO we have any context outside LInux?

Done.


doc/libpmem/pmem_is_pmem.3.md line 37 at r2 (raw file):

int pmem_is_pmem(const void *addr, size_t len);
void *pmem_map_file(const char *path, size_t len, int flags,
	mode_t mode, size_t *mapped_lenp, int *is_pmemp);

Done.


doc/libpmemobj/libpmemobj.7.md line 42 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

Done.


doc/libpmempool/libpmempool.7.md line 41 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

Done.


doc/libpmempool/pmempool_sync.3.md line 34 at r2 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

One line, please

Done.


utils/md2man.sh line 30 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

🙊

Done.

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewed 13 of 13 files at r3, all commit messages.
Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @grom72 and @osalyk)


doc/README line 16 at r2 (raw file):

sources to generate OS-specific variants of man pages and web-based
documentation. The macros are defined in macros.man. Processing is performed
by the ../utils/md2man.sh script.

Suggestion:

To generate web-based documentation or Linux man pages, you need to
have groff and pandoc installed. Processing is performed
by the ../utils/md2man.sh script.

doc/libpmem2/pmem2_source_numa_node.3.md line 52 at r3 (raw file):

The **pmem2_source_numa_node**() can fail with the following errors:

On all systems:

utils/md2man.sh line 54 at r2 (raw file):

	OPTS="$OPTS -DWEB"
	mkdir -p "$(dirname $outfile)"
	m4 $OPTS macros.man $filename | sed -n -e '/---/,$p' > $outfile

Was this part used during the web-dedicated documentation generation?


utils/md2man.sh line 39 at r3 (raw file):

dt=$(date -u -d "@$SOURCE_DATE_EPOCH" +%F 2>/dev/null ||
	date -u -r "$SOURCE_DATE_EPOCH" +%F 2>/dev/null || date -u +%F)
echo $filename | sed -n -e '/# NAME #/,$p' |\

cat

Copy link
Contributor Author

@osalyk osalyk left a comment

Choose a reason for hiding this comment

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

Reviewable status: 21 of 24 files reviewed, 13 unresolved discussions (waiting on @grom72 and @janekmi)


doc/README line 16 at r2 (raw file):

sources to generate OS-specific variants of man pages and web-based
documentation. The macros are defined in macros.man. Processing is performed
by the ../utils/md2man.sh script.

Done.


doc/libpmem2/pmem2_source_numa_node.3.md line 52 at r3 (raw file):

The **pmem2_source_numa_node**() can fail with the following errors:

On all systems:

Done.


utils/md2man.sh line 54 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Was this part used during the web-dedicated documentation generation?

Done.


utils/md2man.sh line 39 at r3 (raw file):

Previously, janekmi (Jan Michalski) wrote…

cat

Done.

Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

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

Reviewed 12 of 13 files at r3, 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @janekmi)

Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @janekmi)

Copy link
Contributor

@grom72 grom72 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @janekmi)

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @osalyk)

@janekmi janekmi merged commit a21dac3 into pmem:master Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation doc/ and other Markdown files sprint goal This pull request is part of the ongoing sprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants