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

Add virtualserialports to python.yaml #29587

Merged
merged 4 commits into from
Jun 17, 2021

Conversation

ggaessler
Copy link
Contributor

@ggaessler ggaessler commented May 20, 2021

Please add the following dependency to the rosdep database.

Package name:

python-virtualserialports-pip
python3-virtualserialports-pip

Package Upstream Source:

https://github.com/ezramorris/PyVirtualSerialPorts

Purpose of using this:

Testing ROS serial device drivers.

Distro packaging links:

Links to Distribution Packages

Only available through PyPI: https://pypi.org/project/PyVirtualSerialPorts/

Signed-off-by: Gabriel Gaessler <gabriel.gaessler@de.bosch.com>
@ggaessler ggaessler requested a review from a team as a code owner May 20, 2021 09:15
Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

I just approved the CI run.

One request though. Unless you're going to be using the python2 version we shouldn't add it. I believe all platforms have moved to python3 now and adding the extra rule that's unused will just require cleanup in the future.

@mabelzhang mabelzhang added the rosdep Issue/PR is for a rosdep key label May 20, 2021
rosdep/python.yaml Outdated Show resolved Hide resolved
Signed-off-by: Gabriel Gaessler <gabriel.gaessler@de.bosch.com>
@ggaessler
Copy link
Contributor Author

I just approved the CI run.

One request though. Unless you're going to be using the python2 version we shouldn't add it. I believe all platforms have moved to python3 now and adding the extra rule that's unused will just require cleanup in the future.

I just put it in for backwards compatibility. I'm not needing it. Just removed the Python2 part.

Signed-off-by: Gabriel Gaessler <gabriel.gaessler@de.bosch.com>
@mabelzhang
Copy link
Contributor

Not sure why the test is failing. Looks like a new test. Could you do a merge from master and see if that fixes it?

@ggaessler
Copy link
Contributor Author

Not sure why the test is failing. Looks like a new test. Could you do a merge from master and see if that fixes it?

Merged it

Copy link
Contributor

@mabelzhang mabelzhang left a comment

Choose a reason for hiding this comment

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

Checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rosdep Issue/PR is for a rosdep key
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants