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 python3-validators to rosdistro #43485

Merged

Conversation

Barry-Xu-2018
Copy link
Contributor

Please add the following dependency to the rosdep database.

Package name:

python3-validators-pip

Package Upstream Source:

https://github.com/python-validators/validators/

Purpose of using this:

For robot AI processing, while using AI python packages like openai or ollama, they will rely on this python package.

Distro packaging links:

Links to Distribution Packages

@Barry-Xu-2018 Barry-Xu-2018 requested a review from a team as a code owner November 13, 2024 06:52
@github-actions github-actions bot added the rosdep Issue/PR is for a rosdep key label Nov 13, 2024
Copy link
Contributor

@sloretz sloretz left a comment

Choose a reason for hiding this comment

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

It looks like this is available from system packages. Please create a system package key instead of a pip key

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

@Barry-Xu-2018 thanks for opening the PR. can you address @sloretz comments?

@Barry-Xu-2018
Copy link
Contributor Author

@Barry-Xu-2018 thanks for opening the PR. can you address @sloretz comments?

Thanks for the reminder. It is done.

@Barry-Xu-2018 Barry-Xu-2018 force-pushed the topic-add-python3-validators-pip branch from f476ffd to 4a2f54c Compare November 18, 2024 05:47
@Barry-Xu-2018
Copy link
Contributor Author

python3-validators wasn't provided in Bionic and Focal version for Ubuntu.
So for the Bionic and Focal versions of Ubuntu, I'll continue using pip to install validators.

@fujitatomoya fujitatomoya requested a review from sloretz November 18, 2024 17:25
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

The bot thinks we may be able to satisfy this on opensuse as well. Once that is added, this should be good to go.

Signed-off-by: Barry Xu <[email protected]>
@Barry-Xu-2018
Copy link
Contributor Author

The bot thinks we may be able to satisfy this on opensuse as well. Once that is added, this should be good to go.

Okay. I added opensuse platform.

@Barry-Xu-2018 Barry-Xu-2018 changed the title Add python3-validators-pip to rosdistro Add python3-validators to rosdistro Nov 19, 2024
@nuclearsandwich
Copy link
Member

python3-validators wasn't provided in Bionic and Focal version for Ubuntu.

Bionic is no longer supported so we don't need to add it. Unless you're actually using the pip package on Focal I don't think it's worth adding there either since its nearly end-of-support itself.

Copy link
Member

@nuclearsandwich nuclearsandwich left a comment

Choose a reason for hiding this comment

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

Thanks for continuing to iterate @Barry-Xu-2018. I have a few recommended changes based on the current state.

Comment on lines 10262 to 10267
bionic:
pip:
packages: [validators]
focal:
pip:
packages: [validators]
Copy link
Member

Choose a reason for hiding this comment

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

Bionic is un-supported so we shouldn't add handling for it. Focal is nearly EOL so I think it's not worth adding unless you're actually using it.

Suggested change
bionic:
pip:
packages: [validators]
focal:
pip:
packages: [validators]

@Barry-Xu-2018
Copy link
Contributor Author

Bionic is no longer supported so we don't need to add it. Unless you're actually using the pip package on Focal I don't think it's worth adding there either since its nearly end-of-support itself.

Get it. Thank you for your information.
I will remove Bionic and Focal.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thanks for sending a pull request to ROS distro!

This is an automated tool that helps check your pull request for correctness.
This tool checks a number of attributes associated with your ROS package and generates a report that helps our reviewers merge your pull request in a timely fashion. Here are a few things to consider when sending adding or updating a package to ROS Distro.
ROS Distro includes a very helpful CONTRIBUTING.md file that we recommend reading if it is your first time submitting a package.
Please also read the ROS Distro review guidelines which summarizes this release process.

ROS Distro Considerations

Package Considerations

Having your package included in a ROS Distro is a badge of quality, and we recommend that package developers strive to create packages of the highest quality. We recommend package developers review the following resources before submitting their package.

Need Help?

Please post your questions to Robotics Stack Exchange or refer to the #infra-help channel on our Discord server.


For changes related to rosdep:

  • ✅ New rosdep keys are named appropriately
  • ✅ Platforms for new rosdep rules are valid

For changes related to yamllint:

  • ✅ All new lines of YAML pass linter checks

@Barry-Xu-2018 Barry-Xu-2018 force-pushed the topic-add-python3-validators-pip branch from ca9ff3f to aaebac2 Compare November 21, 2024 07:06
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

For changes related to rosdep:

  • ✅ New rosdep keys are named appropriately
  • ✅ Platforms for new rosdep rules are valid

For changes related to yamllint:

  • ❌ One or more linter violations were added to YAML files

debian: [python3-validators]
fedora: [python3-validators]
opensuse: [python3-validators]
ubuntu:

Choose a reason for hiding this comment

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

This line does not pass YAML linter checks: trailing spaces

Signed-off-by: Barry Xu <[email protected]>
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

For changes related to rosdep:

  • ✅ New rosdep keys are named appropriately
  • ✅ Platforms for new rosdep rules are valid

For changes related to yamllint:

  • ✅ All new lines of YAML pass linter checks

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

I think we need to have "something" for focal, otherwise the linters will fail. So I think this is good as it is.

@clalancette clalancette dismissed stale reviews from sloretz and nuclearsandwich November 21, 2024 14:11

Stale review.

@clalancette clalancette merged commit 8559bfa into ros:master Nov 21, 2024
5 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.

5 participants