StoglRobotics / ros_team_workspace

RosTeamWorkspace is a framework for boosting collaboration in teams when developing software for robots using Robot Operating System (ROS1 and ROS2).
https://rtw.stoglrobotics.de
Apache License 2.0
79 stars 17 forks source link

[python cli] 'rtw docker enter' #126

Closed muritane closed 1 year ago

muritane commented 1 year ago

just run this cd ros_team_workspace/rtwcli/ && pip3 install -r requirements.txt and start new terminal as sourcing is already done in _ros_teamworkspace/setup.bash

@destogl pep257 is commented out temporarily as the import of Set raises Exception: cannot import name 'Set' from 'collections', is there a repo that supports newer python versions?

mergify[bot] commented 1 year ago

This pull request is in conflict. Could you fix it @muritane?

mamueluth commented 1 year ago

Nice work! Looks good:) I would however advise to first create only the cli and call the existing bash scripts with the expected arguments. I would not start recreating/adding logic in the python cli. This should in my opinion be done in a separate step while writing tests at the same time. Moreover, in my opinion, we have a lot of common functionality which can be extracted from the scripts and collected at one point. I would keep the cli as thin as possible and not scatter to much logic all over the place (otherwise we end up with lot of duplicate logic.)

mamueluth commented 1 year ago

Have to give it a closer review

destogl commented 1 year ago

@destogl pep257 is commented out temporarily as the import of Set raises Exception: cannot import name 'Set' from 'collections', is there a repo that supports newer python versions?

I fixed that.

muritane commented 1 year ago

will be split into multiple prs