-
Notifications
You must be signed in to change notification settings - Fork 421
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
Toward enabling dashboards on CI #328
Toward enabling dashboards on CI #328
Conversation
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.
Sorry, I think we missed this PR.
+@SeanCurtis-TRI do you want to review this?
@jamiesnape, there are merge conflicts that need smoothing over.
Reviewed 2 of 4 files at r1.
Reviewable status: 2 of 4 files reviewed, all discussions resolved
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 hadn't missed this -- at the time @jamiesnape created this, I'd expressed the desire that this stuff should happen after we do the next release based on the current paradigm. And then after 0.6.0 is out, we can make all of the big changes related to the discussed overhaul.
What we really need is to figure out what is preventing us from releasing 0.6.0 and then resolving those issues.
Reviewable status: 2 of 4 files reviewed, all discussions resolved (waiting on @SeanCurtis-TRI)
I don't think this particular one necessarily needs to wait, however. The others, maybe. I could remove the deprecation warning on the |
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.
My ignorance of CMake's inner workings have probably led to a bunch of novice-style questions. But from what I'm able to infer, .
Reviewed 4 of 4 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved
.gitignore, line 30 at r2 (raw file):
# POSSIBILITY OF SUCH DAMAGE. # Author: Jeongseok Lee <[email protected]>
BTW The copyright holder, year, and author combination surprised me.
CMakeLists.txt, line 38 at r2 (raw file):
if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES) set(CMAKE_BUILD_TYPE Release CACHE STRING "Choose the type of build; options are Debug Release RelWithDebInfo MinSizeRel."
BTW This appears to both set the default build type as well as prompt the user to do so.
CTestConfig.cmake, line 35 at r2 (raw file):
# Author: Jamie Snape, Kitware, Inc. set(CTEST_PROJECT_NAME fcl)
BTW This stuff is merely configuration, right? Until it's actually exercised by some CI server, it sets here twiddling its thumbs?
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.
Reviewable status: complete! all files reviewed, all discussions resolved
.gitignore, line 30 at r2 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
BTW The copyright holder, year, and author combination surprised me.
That is what git reports.
CMakeLists.txt, line 38 at r2 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
BTW This appears to both set the default build type as well as prompt the user to do so.
No, if the user sets the build type this never gets triggered. The text is just there to maintain the standard comment in the cache.
CTestConfig.cmake, line 35 at r2 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
BTW This stuff is merely configuration, right? Until it's actually exercised by some CI server, it sets here twiddling its thumbs?
Yes.
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.
Thanks for taking the time to help out my ignorance.
Reviewable status: complete! all files reviewed, all discussions resolved
Part 1 of many. Relates #329.
This change is