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.

Sign up for GitHub

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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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