Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

dependency check #25

Merged
merged 4 commits into from
Mar 5, 2019
Merged

dependency check #25

merged 4 commits into from
Mar 5, 2019

Conversation

kahlering
Copy link
Contributor

No description provided.

Copy link
Member

@ichumuh ichumuh left a comment

Choose a reason for hiding this comment

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

general remark: use rospy.logwarn() etc instead of print. I think logwarn is fine for all the prints here.

dependencies.txt Outdated
qpoases==3.1.0
giskard_msgs==0.1.0
py_trees_ros==0.5.14
py_trees==0.6.1
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a bit too restrictive. Can you do an upper and lower limit, something like 1.15.4 < numpy < 2.0? Some libraries push the first number only if they have breaking api changes. So a restriction like my example would be nice for all libs. Only for pybullet we should require a specific version, its also missing in this list.

elif version_m[i] > version_d[i]:
print("found ROS package " + l[0][:-1] + "==" + str(
m.version) + " but " + name + " is required")
Copy link
Member

Choose a reason for hiding this comment

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

Please always use single quotes for strings, so that it's consistent with the rest of my code. And use '{}'.format(), to build strings.

r = rospkg.RosPack()
try:
l = name.split('=')
Copy link
Member

Choose a reason for hiding this comment

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

Please use more descriptive variable names.

@ichumuh ichumuh merged commit be3cde8 into SemRoCo:devel Mar 5, 2019
@ichumuh
Copy link
Member

ichumuh commented Mar 5, 2019

thx

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants