-
Notifications
You must be signed in to change notification settings - Fork 8
build(cmake): Add support for building library targets that contain source files and linked libraries. #36
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
Merged
Bill-hbrhbr
merged 17 commits into
y-scope:main
from
Bill-hbrhbr:add-support-for-libs-with-src
Mar 16, 2025
Merged
Changes from 2 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
33bf92d
Add ability to create libraries with source file components
Bill-hbrhbr 1c47e59
Add deps args
Bill-hbrhbr 8015150
Use TF rather than 01
Bill-hbrhbr 352040a
Address coderabbitai concerns
Bill-hbrhbr 467182c
Merge branch 'main' into add-support-for-libs-with-src
Bill-hbrhbr e81a086
Update library cmakes
Bill-hbrhbr f339ef8
use return propagate
Bill-hbrhbr 0641dba
remove unused HEADERS arg
Bill-hbrhbr 40c2d1f
Address review concern. Rename most args
Bill-hbrhbr 2f508d7
Update comment
Bill-hbrhbr 31f8dd1
Change cpp_library() arg names
Bill-hbrhbr 90a6604
Clang format
Bill-hbrhbr 37aaa13
Update clang-format/tidy tool versions
Bill-hbrhbr b2298ae
Update lint requirements
Bill-hbrhbr 2c1f968
Undo irrelevant change
Bill-hbrhbr a2290ea
Merge branch 'main' into add-support-for-libs-with-src
Bill-hbrhbr f1b792d
Reflow comment
Bill-hbrhbr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think everything works even if we don't separate/distinguish headers and source files, so I think just
SOURCESis fine. If it is important to separate them, then maybe we should concatenate them and pass both tocheck_if_header_only_libraryto avoid user error (currently if you put a cpp file inHEADERSI think things will break)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not important to separate now, but will make a difference when we only want to install public headers into install library paths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a second thought, I've decided to keep them split and renamed the params for more clarity. I've also enforced a check of the public header list.