-
Notifications
You must be signed in to change notification settings - Fork 45
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
Give TribitsSimpleExampleApp the utils program to match TribitsOldSimpleExampleApp (#299) #440
Give TribitsSimpleExampleApp the utils program to match TribitsOldSimpleExampleApp (#299) #440
Conversation
Not sure why I was using the old style of add_test() but this was not a good example of how to use raw CMake.
This adds a 'util' program that only uses the libs in the WithSubpackagesB package. This shows how to select just a subset of libraries from a package read in after calling find_package(<ParentProject> ...). This now makes the TribitsSimpleExampleApp and TribitsOldSimpleExampleApp projects almost identical where the former uses modern CMake and the modern TriBITS-generated targets and the latter works with old and new versions of TriBITS. Also: * Switched to add_test(NAME ... COMMAND ...) * Updated test drivers for the new 'util' test
@marcinwrobel1986 and @mperrinel, can you both please post a post-merge review of this, even if you don't suggest any changes? To do that, go to the "Files changed" tab and in the upper right click on the green "Review Changes" button. But before you post the review, you can, of course, add comments to the code by clicking "Start a review". |
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 haven't found any problems. Nice one, thanks!
…#442) Add support for subpackage-specific compiler flags (#440 Also see, trilinos/Trilinos#10111
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.
The PR is ok for me.
Only found some potential missing links
@@ -17,3 +20,7 @@ After building and installing TribitsExampleProject under | |||
|
|||
ctest | |||
``` | |||
|
|||
NOTE: The version of this project that demonstrates using the old interface | |||
using variables that works with much older versions of TriBITS is given in |
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 don't think the NOTE is very clear. What do you think about:
The version of this project that demonstrates how to use the old interface, with variables that work with much older versions of TriBITS, is given in
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.
@mperrinel, sure, just put in a PR with your suggested updates.
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.
@mperrinel, just to summarize, I think it is generally considered best practice to hyperlink the first mention of something on a given page or paragraph. That makes the formatted text look cleaner but still provides the needed links/definitions. |
This makes the TribitsSimpleExampleApp and TribitsOldSimpleExampleApp identical except the former uses the new targets namespaced modern TriBITS targets and the latter uses the old backward-compatible variables that work with new and old TriBITS-based TribitsExampleProject.
See the individual commits and commit logs for more details.