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 ifcfg-pip #22071

Closed
wants to merge 4 commits into from
Closed

Add ifcfg-pip #22071

wants to merge 4 commits into from

Conversation

claireyywang
Copy link
Contributor

@claireyywang claireyywang commented Aug 21, 2019

This package is used in ros2doctor network feature to check network configuration.
Package repo can be found at https://github.com/ftao/python-ifcfg, and it is compatible with Python2 and Python3.
Web index: https://pypi.org/project/ifcfg/, no platform specific package index found.
Explicit vendor package can be found at https://github.com/ros2/ifcfg_vendor

Claire Wang added 4 commits August 14, 2019 10:53
Signed-off-by: claireyywang <clairewang@openrobotics.org>
Signed-off-by: claireyywang <clairewang@openrobotics.org>
@claireyywang claireyywang requested a review from a team as a code owner August 21, 2019 22:33
claireyywang pushed a commit to ros2/ros2cli that referenced this pull request Aug 21, 2019
waiting for rosdistro PR approval ros/rosdistro#22071
@claireyywang
Copy link
Contributor Author

working on a deb vendor package

@@ -154,6 +154,19 @@ gunicorn:
fedora: [python-gunicorn]
gentoo: [www-servers/gunicorn]
ubuntu: [gunicorn]
ifcfg-pip:
Copy link
Member

Choose a reason for hiding this comment

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

Since we're using this for ROS 2 we'll want the key name for the platform installer to be python3-ifcfg. I think this key name is fine for pip. An alternative might be python3-ifcfg-pip but since the pip key is not python3 specific I don't feel a preference for it.

@nuclearsandwich
Copy link
Member

working on a deb vendor package

I like explicit vendor packages rather than packaging things off the farm and importing them as we do for opensplice and lark-parser but let's check in with some other folks tomorrow to decide what to do for ifcfg. I have also added this to my big list of things to try and get upstreamed into Debian for future ROS 2 releases.

@nuclearsandwich nuclearsandwich requested a review from a team August 22, 2019 03:56
claireyywang pushed a commit to ros2/ros2cli that referenced this pull request Aug 26, 2019
* add network checks and report

Signed-off-by: claireyywang <clairewang@openrobotics.org>

* network shenanigens

Signed-off-by: claireyywang <clairewang@openrobotics.org>

* network shenanigens

Signed-off-by: claireyywang <clairewang@openrobotics.org>

* network shenanigens

Signed-off-by: claireyywang <clairewang@openrobotics.org>

* add network check and report

Signed-off-by: claireyywang <clairewang@openrobotics.org>

* update code format

Signed-off-by: claireyywang <clairewang@openrobotics.org>

* revised code format

Signed-off-by: claireyywang <clairewang@openrobotics.org>

* add ifcfg-pip rosdep key 

waiting for rosdistro PR approval ros/rosdistro#22071

* added rosdep key ifcfg-pip

Signed-off-by: claireyywang <clairewang@openrobotics.org>

* revise code

Signed-off-by: claireyywang <clairewang@openrobotics.org>
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.

If we want this to be a core tool dependency I think that using a vendor package is better than using pip, because we can't use pip as a core dependency when we're packaging.

And if we're going to vendor it then we I think it's better not to have conflicting pip rules for it.

@claireyywang
Copy link
Contributor Author

Closing as a ifcfg debian package is made available; ifcfg_vendor is archived in ros2.repos. Will open a new PR if needed.

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.

None yet

3 participants