-
Notifications
You must be signed in to change notification settings - Fork 30
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
Fix OS X dependencies, remove ubuntu disco, update C++ standard #570
Conversation
disco
and fix OS X segfaults)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.
Minor nits. I think we should also update the README to reflect the new C++ standard.
@@ -3,7 +3,7 @@ brew 'dartsim' | |||
brew 'eigen' | |||
brew 'libmicrohttpd' | |||
brew 'ompl' | |||
brew 'tinyxml2' | |||
brew 'tinyxml' | |||
brew 'yaml-cpp' | |||
|
|||
# Install dartsim dependencies explicitly because, for some unknown reasons, |
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 being able to automatically pull in transitive dependencies is incredibly annoying. It really seems like there should be an obvious way to fix this. Can we add an issue to do so?
@jslee02 do you remember whether this problem was just with Travis CI + Homebrew? Or whether it's a plain Homebrew issue?
In the worst case, it seems like we should be able to do something like brew deps dartsim
from Travis to generate this list of dependencies that should be installed.
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.
Added issue #571
Maybe we can unblock PRs by pushing this and continue discussion there?
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.
Yep, absolutely.
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.
@jslee02 do you remember whether this problem was just with Travis CI + Homebrew? Or whether it's a plain Homebrew issue?
I didn't investigate further but just this workaround. Probably it was just Homebrew. Not sure if it's fixed or now. 😞
Currently, all tests segfault after main on OS X. This was due to a missing dart dependency:
tinyxml
.In addition, Ubuntu disco is no longer supported, so that has been updated to the latest LTS release (focal).
Finally, due to defaults in both bionic and focal, this updates the standard to C++14. This matches the DART standard, allows for now-idiomatic function (e.g.
std::make_unique
), and its where we should be heading for a stable release.Before creating a pull request
make format
Before merging a pull request
CHANGELOG.md