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

MIL COMMON: clean redundant packages and prepare for great merge & sonar OS #216

Merged
merged 9 commits into from
Apr 11, 2017

Conversation

kev-the-dev
Copy link
Contributor

  • remove all/part of sub8_vision_lib sub8_ros_tools uf_common and passive sonar packages
  • make namespace changes to use mil_common versions of the above where possible

Why I think Nothings broken

  • Project compiles running my branch of mil_common and this PR
  • git grep for the following strings return nothing: ""sub8_tools" "sub8_ros_tools" "sub8_mil_tools" "uf_common"
  • running sub8_missions run_common list shows all missions without import errors

Obviously don't merge until uf-mil/mil_common#15 is merged and uf-mil/mil_common#28

@kev-the-dev kev-the-dev changed the title MIL COMMON: clean redundent packages and prepare for great merge MIL COMMON: clean redundent packages and prepare for great merge & sonar OS Apr 11, 2017
Copy link
Member

@DSsoto DSsoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting a couple of minor changes.

#from sub8_alarm import AlarmListener, AlarmBroadcaster
from ros_alarms import AlarmBroadcaster, AlarmListener
import sub8_ros_tools as sub8_utils
import mil_ros_tools as sub8_utils
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be imported as sub8_utils.

@@ -2,7 +2,7 @@
import sys
import rospy
import nav_msgs.msg as nav_msgs
import sub8_ros_tools as sub8_utils
import mil_ros_tools as sub8_utils
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be imported as sub8_utils.

@@ -12,7 +12,7 @@
from scipy import linalg

import actionlib
import uf_common.msg as uf_common_msgs
import mil_msgs.msg as mil_msgs_msgs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It soesnt make sense to import as a name and that name is longer. How about:

from mil_msgs.msg import MoveToAction, MoveToGoal

@@ -10,7 +10,7 @@ import yaml
import roslib
import rosbag
from tf import transformations
import sub8_ros_tools as sub8_utils
import mil_ros_tools as sub8_utils
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

@@ -1,6 +1,6 @@
import numpy as np
import rospy
import sub8_ros_tools as sub8_utils
import mil_ros_tools as sub8_utils
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

Ran the following commands:
git grep -l 'sub8_ros_tools' | xargs sed -i 's/sub8_ros_tools/mil_ros_tools/g'
git grep -l 'uf_common' | xargs sed -i 's/uf_common/mil_msgs/g'
git grep -l 'sub8_misc_tools' | xargs sed -i 's/sub8_misc_tools/mil_misc_tools/g'
git grep -l 'sub8_vision_lib' | xargs sed -i 's/sub8_vision_lib/mil_vision_lib/g'
I'll see what it breaks in following commits
Remove sub8_ros_tools because it is a duplicate of mil_tools
Remove uf_common beacuase it is a duplicate of mil_msgs
@sentree
Copy link
Member

sentree commented Apr 11, 2017

This built on the CI and passed all unit tests except the ros_alarms test that always fails.
-- Your friendly temporary CI guy

@DSsoto DSsoto changed the title MIL COMMON: clean redundent packages and prepare for great merge & sonar OS MIL COMMON: clean redundant packages and prepare for great merge & sonar OS Apr 11, 2017
@DSsoto DSsoto removed the request for review from mattlangford April 11, 2017 05:45
@DSsoto DSsoto merged commit 5d169e9 into uf-mil:master Apr 11, 2017
@kev-the-dev kev-the-dev deleted the great-integration branch May 23, 2017 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants