Skip to content

Add '--disable-doc' option#154

Merged
mtkaczyk merged 1 commit into
md-raid-utilities:masterfrom
bigunclemax:add_disable_doc_option
Aug 24, 2023
Merged

Add '--disable-doc' option#154
mtkaczyk merged 1 commit into
md-raid-utilities:masterfrom
bigunclemax:add_disable_doc_option

Conversation

@bigunclemax
Copy link
Copy Markdown
Contributor

Introduce a configure option to disable documentation installation in case if it is not required.

Copy link
Copy Markdown
Member

@mtkaczyk mtkaczyk left a comment

Choose a reason for hiding this comment

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

I'm ok with the change, please fix issues and we can merge it.
Thanks!

Comment thread configure.ac
Comment thread configure.ac Outdated
@bigunclemax bigunclemax force-pushed the add_disable_doc_option branch from eadf4b0 to 82510a7 Compare August 21, 2023 15:20
@mtkaczyk
Copy link
Copy Markdown
Member

one small nit- please add description to README.md.

Thanks!

@tasleson
Copy link
Copy Markdown
Collaborator

@bigunclemax @mtkaczyk If possible I believe it would be good to keep the default behavior the same so that packagers could drop in a new release without requiring changes to the packaging itself. So in my opinion this should be something like --enable-no-doc so that for the default, the documentation is included. All the other options added recently didn't change the default installation behavior.

Introduce a configure option to disable documentation installation
in case if it is not required.

Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
@bigunclemax bigunclemax force-pushed the add_disable_doc_option branch from 82510a7 to 45b2c7e Compare August 22, 2023 20:48
@mtkaczyk
Copy link
Copy Markdown
Member

@bigunclemax @mtkaczyk If possible I believe it would be good to keep the default behavior the same so that packagers could drop in a new release without requiring changes to the packaging itself. So in my opinion this should be something like --enable-no-doc so that for the default, the documentation is included. All the other options added recently didn't change the default installation behavior.

For library we have:

AC_ARG_ENABLE([library],
    [AS_HELP_STRING([--enable-library],
        [enable building ledmon library])],
    [with_library=${enableval}],
    [with_library=no])

For doc we reversed it:

AC_ARG_ENABLE([doc],
    [AS_HELP_STRING([--disable-doc],
        [do not install ledmon documentation])],
    [with_doc=${enableval}],
    [with_doc=yes])

So, unfortunately the default behavior will be different but I still preffer that than way you proposed because the alternative form supported by macro AC_ARG_ENABLE --disable-no-doc is terrible to read :)

@mtkaczyk mtkaczyk merged commit 25eb432 into md-raid-utilities:master Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants