Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

cmake: add options for binary build and install targets #342

Merged
merged 2 commits into from
Feb 6, 2024
Merged

cmake: add options for binary build and install targets #342

merged 2 commits into from
Feb 6, 2024

Conversation

clwluvw
Copy link
Contributor

@clwluvw clwluvw commented Jan 29, 2024

  • make binary build optional
  • generate install targets optionally

@clwluvw
cmake: make binary build optional
f191124 
Signed-off-by: Seena Fallah <seenafallah@gmail.com>
oschwald
oschwald requested changes Feb 5, 2024
View reviewed changes
Copy link
Member

@oschwald oschwald left a comment

Choose a reason for hiding this comment

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

Thanks! This looks good and I could see how disabling the building of the binaries could be useful. I am curious what the motivation is behind disabling the install target; it seems fine, but I am a bit curious.

I had one small comment.

CMakeLists.txt Outdated Show resolved Hide resolved
@clwluvw
cmake: generate install targets optionally
878e2ca 
Signed-off-by: Seena Fallah <seenafallah@gmail.com>
@clwluvw
Copy link
Contributor Author

clwluvw commented Feb 6, 2024 •
edited
Loading

I am curious what the motivation is behind disabling the install target

I found it when I use the library as a submodule or via FetchContent (basically the add_subdirectory function) and then the installation targets would be useless and can lead to installation in packaging processes while it's not needed.

@oschwald oschwald merged commit 5c5d875 into maxmind:main Feb 6, 2024
15 checks passed
@oschwald
Copy link
Member

oschwald commented Feb 6, 2024

Thank you!

oschwald added a commit that referenced this pull request Feb 6, 2024
@oschwald
Add changes entry for #342
573bac5
horgh added a commit that referenced this pull request Feb 6, 2024
@horgh
Merge pull request #343 from maxmind/greg/changelog-342
a7c9f3b 
Add changes entry for #342
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@oschwald oschwald oschwald approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@clwluvw @oschwald