search

moveit / srdfdom

Semantic Robot Description Format
BSD 3-Clause "New" or "Revised" License
12 stars 68 forks source link

Add SRDFWriter to this repo #14

Closed velveteenrobot closed 8 years ago

velveteenrobot commented 8 years ago

Fixes issue: https://github.com/ros-planning/moveit/issues/66 from MoveIt.

velveteenrobot commented 8 years ago

Merge this before https://github.com/ros-planning/moveit/pull/142.

rhaschke commented 8 years ago

Merged. @130s Could you prepare a release of srdfdom to unblock https://github.com/ros-planning/moveit/pull/142? Thanks to @velveteenrobot.

rhaschke commented 8 years ago

Hm. Now that I merged, Travis fails in MoveIt. I will add a travis config to srdfdom too.

rhaschke commented 8 years ago

Fixed srdfdom. Updated .travis.yml to MoveIt's one. Travis is back to normal: https://travis-ci.org/rhaschke/srdfdom @davetcoleman Do you have an idea, why https://travis-ci.org/ros-planning/srdfdom didn't build for ages?

130s commented 8 years ago

Could you prepare a release of srdfdom to unblock ros-planning/moveit#142?

Let me know when it's ready.

Do you have an idea, why https://travis-ci.org/ros-planning/srdfdom didn't build for ages?

Has travis been on for this repo (I see now it's on at http://travis-ci.org/profile/ros-planning though).

rhaschke commented 8 years ago

Do you have an idea, why https://travis-ci.org/ros-planning/srdfdom didn't build for ages?

Has travis been on for this repo (I see now it's on at http://travis-ci.org/profile/ros-planning though).

Yes, travis was configured to be on, but it nevertheless didn't triggered builds on pushes or PRs. I just noticed that the detailed settings were configured to be off! Fixed.

rhaschke commented 8 years ago

OK, Travis triggers again: https://travis-ci.org/ros-planning/srdfdom/builds/154885328 @130s: IMHO you could release now. @davetcoleman?

davetcoleman commented 8 years ago

This is a violation of our review policy and standard software procedures - why are you committing some much unreviewed stuff to this repo?

screenshot from 2016-08-25 00-42-00

rhaschke commented 8 years ago

I tried to hot-fixed the merge, which broke Travis of the main Moveit repo. That's why some urgent action was required. Before doing the original merge, I of course tested locally, and surprisingly I didn't observe any issues. Still not sure why. Unfortunately, Travis was disabled for this repo, such that the issues didn't show up here automatically.

130s commented 8 years ago

Will make a release 0.3.2 now.

Yeah, I have to agree with @davetcoleman about direct commits. Maybe @rhaschke you could have hot-fixed and then if that worked you revert and open a PR?

v4hn commented 8 years ago

I also hot-fixed things in the past and I don't think it's fatal. But hot-fixes should be limited to single commits.

a1dd5324cf15117fd6818f4c3e9d6b3ea2f7f7a0 is definitely no "hot-fix" and enabling proper travis support isn't either.

rhaschke commented 8 years ago

a1dd532 is definitely no "hot-fix"

What else? The call to exit() and the use of ros/console.h, which wasn't declared a dependency, broke building of the package. For this reason unrelated PRs of the main moveit repo failed on Travis: https://travis-ci.org/ros-planning/moveit/pull_requests (237-238). Because of my hot-fix, the down-time was kept as short as possible.

v4hn commented 8 years ago

On Thu, Aug 25, 2016 at 05:41:40PM -0700, Robert Haschke wrote:

The call to exit() and the use of ros/console.h, which wasn't declared a dependency, broke building of the package.

Then why didn't you write the explanation in the commit message? :-) Please do that next time.

130s commented 8 years ago

0.3.2 along with this change now made to shadow repo for Indigo and JK.