search

ament / ament_index

Apache License 2.0
13 stars 27 forks source link

Target is looking for a thread library so it compiles on linux system… #19

Closed firesurfer closed 7 years ago

firesurfer commented 8 years ago

…s that don't link to pthread by default.

firesurfer commented 7 years ago

I finaly found some time to fix the given issues. Could you have a look at my last commit ?

dhood commented 7 years ago

@dirk-thomas I believe the commits on this branch that are from master are because ros2/master was merged into this PR as opposed to this PR being rebased against it. Just the two new commits (the first and the last) will be applied if this gets merged - the other hashes are the same so git knows to not re-apply them (I've done the same in some of my PRs before)

CI for this and https://github.com/ament/ament_cmake/pull/78 together:

Linux: Build Status OS X: Build Status Windows: Build Status

The instability were unrelated and have been addressed (https://github.com/ros2/rclpy/commit/330512595c3a3cb64c3eae3825533292a15b84bc and https://github.com/ros2/rosidl/commit/bf854d971f2afa95e194a795c0da4d57d9d8a683).

so it seems like just the whitespace comment that needs to be addressed

dirk-thomas commented 7 years ago

The diff in the PR should only show actual changes. Maybe the history needs to be rewritten. But otherwise it is just more complicated to see and review the actual changes. Therefore I think this should be done anyway.

firesurfer commented 7 years ago

Could you perhaps point out how to rebase this. If I checkout my forked master branch and do a git rebase -i origin/master it just shows me my own three commits. On my other PR for ament_cmake I did a git rebase -i HEAD~ and removed the merged commits. But after pushing github closed my PR automatically.

tfoote commented 7 years ago

Your rebase command against origin/master looks right. Though you should check what your remotes are configured as. Make sure that origin is pointing to this repo and not your fork. Also make sure you git fetch origin before rebasing as that might be the difference between your view and github's view.

dirk-thomas commented 7 years ago

Is this not covered by ament/ament_cmake#82 already?

firesurfer commented 7 years ago

I think you are right. When I created these PRs I looked step for step where the compilation failed. Apparently I didn't recheck that code.Therefore I'm closing this PR .