Skip to content
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

cob_manipulation: 0.7.5-1 in 'noetic/distribution.yaml' [bloom] #27682

Merged
merged 1 commit into from
Dec 21, 2020

Conversation

fmessmer
Copy link
Contributor

@fmessmer fmessmer commented Dec 8, 2020

Increasing version of package(s) in repository cob_manipulation to 0.7.5-1:

cob_collision_monitor

* Merge pull request #148 <https://github.com/ipa320/cob_manipulation/issues/148> from fmessmer/test_noetic
  test noetic
* Bump CMake version to avoid CMP0048 warning
* Merge pull request #149 <https://github.com/ipa320/cob_manipulation/issues/149> from fmessmer/fix_catkin_lint
  fix catkin_lint
* fix catkin_lint
* Contributors: Felix Messmer, fmessmer

cob_grasp_generation

* Merge pull request #148 <https://github.com/ipa320/cob_manipulation/issues/148> from fmessmer/test_noetic
  test noetic
* ROS_PYTHON_VERSION conditional dependency
* use setuptools instead of distutils
* Bump CMake version to avoid CMP0048 warning
* Contributors: Felix Messmer, fmessmer

cob_lookat_action

* Merge pull request #148 <https://github.com/ipa320/cob_manipulation/issues/148> from fmessmer/test_noetic
  test noetic
* conditional depend for orocos-kdl
* Bump CMake version to avoid CMP0048 warning
* Merge pull request #149 <https://github.com/ipa320/cob_manipulation/issues/149> from fmessmer/fix_catkin_lint
  fix catkin_lint
* fix catkin_lint
* Merge pull request #146 <https://github.com/ipa320/cob_manipulation/issues/146> from fmessmer/lookat_improve_fjt_goal
  [cob_lookat] improve fjt goal
* do not set tolerances
* set velocities and accelerations to 0.0 for traj_point
* Contributors: Felix Messmer, fmessmer

cob_moveit_bringup

* Merge pull request #148 <https://github.com/ipa320/cob_manipulation/issues/148> from fmessmer/test_noetic
  test noetic
* Bump CMake version to avoid CMP0048 warning
* Merge pull request #149 <https://github.com/ipa320/cob_manipulation/issues/149> from fmessmer/fix_catkin_lint
  fix catkin_lint
* fix catkin_lint
* Contributors: Felix Messmer, fmessmer

cob_moveit_interface

* Merge pull request #148 <https://github.com/ipa320/cob_manipulation/issues/148> from fmessmer/test_noetic
  test noetic
* ROS_PYTHON_VERSION conditional dependency
* use setuptools instead of distutils
* Bump CMake version to avoid CMP0048 warning
* Contributors: Felix Messmer, fmessmer

@nuclearsandwich
Copy link
Member

holding for the upcoming Noetic sync

@fmessmer
Copy link
Contributor Author

fmessmer commented Dec 10, 2020

@nuclearsandwich
oh...it would be really cool to get this out in the sync...as this is the final repo to be released for care-o-bot...any chance?

@nuclearsandwich
Copy link
Member

@fmessmer can you please comment on the sync thread I linked above with that request. It will be @sloretz who ultimately makes the decision.

@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/preparing-for-noetic-sync-2020-12-14/17835/2

@sloretz
Copy link
Contributor

sloretz commented Dec 14, 2020

Shoot, I missed this before the weekend. I would have been fine with putting it in since it's a brand new package, but I'd rather not delay the sync now to wait for these packages to build on the build farm.

@fmessmer
Copy link
Contributor Author

@sloretz well now that the sync is out...is there anything else that is blocking this PR from being merged?

@sloretz
Copy link
Contributor

sloretz commented Dec 15, 2020

@sloretz well now that the sync is out...is there anything else that is blocking this PR from being merged?

Nothing blocking it. I do notice the upstream repository doesn't have any LICENSE files. The review guidelines say to require them of new packages, but maybe it would be fine to merge as is since it's already been released to earlier distros.

@nuclearsandwich, @clalancette, thoughts?

@nuclearsandwich
Copy link
Member

nuclearsandwich commented Dec 15, 2020

maybe it would be fine to merge as is since it's already been released to earlier distros.

I like using our review guidelines to iteratively improve the situation from release to release. Ideally we would hold it for that.

@fmessmer
Copy link
Contributor Author

fmessmer commented Dec 15, 2020

none of the care-o-bot repos has any such file and this has not been requested for any of those repos...this pr is the very final pr for the care-o-bot series...this repo has already been released for kinetic, melodic for years...

maybe this LICENSE policy should be checked/enforced during bloom-release already?

also, since I'm no longer working for Fraunhofer IPA who hosts these repos, I fear this will take a long while to settle - if at all
I'm voluntarily updating and releasing these packages (in my freetime)

@fmessmer
Copy link
Contributor Author

fmessmer commented Dec 17, 2020

so this PR will not be merged then? @nuclearsandwich @sloretz @clalancette
adding a license is nothing I can influence alone...
...but this PR is blocking further noetic migration efforts...

I'd just need a decision because then I would work around this missing release...

EDIT:
issue to track this for next release cycle: ipa320/care-o-bot#60

@jacobperron
Copy link
Contributor

I have been interpreting the review guidelines as only blocking for brand new packages (not packages that already exist in a ROS distribution). In this case, I don't think we need to block on having a LICENSE file. Of course, if possible it would be good to get a LICENSE file added. Thoughts?

@jacobperron
Copy link
Contributor

Given the circumstances, I'm going ahead with merging this. Hopefully the ticket for adding a LICENSE file will be resolved before the next release.

@jacobperron jacobperron merged commit 84b690c into ros:master Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants