Skip to content

meson: Add option for installing tests#511

Merged
debarshiray merged 1 commit intocontainers:masterfrom
martymichal:meson-install-tests-option
Aug 13, 2020
Merged

meson: Add option for installing tests#511
debarshiray merged 1 commit intocontainers:masterfrom
martymichal:meson-install-tests-option

Conversation

@martymichal
Copy link
Copy Markdown
Member

@martymichal martymichal commented Jul 23, 2020 •
edited
Loading

This adds a new option to Meson called with_tests making it possible to install whole 'test' subdir into the system's data dir (usually /share or /usr/share).

@martymichal martymichal added 3. Enhancement Improvement to an existing feature 6. Minor Change Should not cause breakage labels Jul 23, 2020
@softwarefactory-project-zuul
Copy link
Copy Markdown

softwarefactory-project-zuul bot commented Jul 23, 2020

Build failed.

@martymichal martymichal force-pushed the meson-install-tests-option branch from c735097 to 2b1ae1b Compare July 23, 2020 21:32
@softwarefactory-project-zuul
Copy link
Copy Markdown

softwarefactory-project-zuul bot commented Jul 23, 2020

Build failed.

debarshiray
debarshiray reviewed Aug 12, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@debarshiray debarshiray left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

Do we really need a separate build option? I'd always install the tests unconditionally, unless there are cases where they might cause problems.

You know, the usual spiel about less being better when it comes to options. :)

Sometimes projects (eg., in GNOME) have an option for disabling documentation. That's done to help continuous integration and delivery systems like GNOME Continuous, because building the entire documentation generation stack (including gtk-doc and all its Docbook underpinnings) can be quite onerous. So it's more practical to disable generating the documentation. It doesn't look like we have such problems here.

Oh, and it would nice to link to this PR from the Git commit message.

debarshiray pushed a commit to martymichal/toolbox that referenced this pull request Aug 13, 2020
@martymichal @debarshiray
build: Install the tests
9bd2ae4 
Installing the tests will let downstream distributors like Fedora run
them as part of their build and CI systems.

containers#511
@debarshiray debarshiray force-pushed the meson-install-tests-option branch from 2b1ae1b to 9bd2ae4 Compare August 13, 2020 09:33
@martymichal @debarshiray
build: Install the tests
f246d20 
Installing the tests will let downstream distributors like Fedora run
them as part of their build and CI systems.

containers#511
@debarshiray debarshiray force-pushed the meson-install-tests-option branch from 9bd2ae4 to f246d20 Compare August 13, 2020 09:34
@debarshiray debarshiray merged commit f246d20 into containers:master Aug 13, 2020
@martymichal
Copy link
Copy Markdown
Member Author

martymichal commented Aug 13, 2020 •
edited
Loading

My thinking behind making this a build option was that the tests are not something that is necessary for running Toolbox, thus they don't need to be enabled by default. But I believe you're right about this and the option is not really necessary.

I'll add the link to the PR (I forgot to do that when I added CONTRIBTING.md...).

EDIT: I just realized you already merged it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@debarshiray debarshiray debarshiray left review comments

Assignees

No one assigned

Labels

3. Enhancement Improvement to an existing feature 6. Minor Change Should not cause breakage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@martymichal @debarshiray