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.

Sign up for GitHub

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

Jump to bottom

Fix a number of warnings on Ubuntu 24.04. #289

Merged
merged 3 commits into from Feb 1, 2024
Merged

Fix a number of warnings on Ubuntu 24.04. #289

merged 3 commits into from Feb 1, 2024

Conversation

clalancette
Copy link
Contributor

In particular:

  1. Get rid of the fallback path for argcompleter. It isn't necessary anymore since all versions of argcompleter support these, and it was confusing mypy.
  2. Add in a proper Optional annotation.
  3. Use the newer importlib_resources.file API when it is available.

@clalancette
Fix a number of warnings on Ubuntu 24.04.
cab6689 
In particular:
1.  Get rid of the fallback path for argcompleter.  It
    isn't necessary anymore since all versions of argcompleter
    support these, and it was confusing mypy.
2.  Add in a proper Optional annotation.
3.  Use the newer importlib_resources.file API when it is
    available.

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
ahcorde
ahcorde approved these changes Jan 31, 2024
View reviewed changes
Copy link

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@clalancette
Copy link
Contributor Author

CI is failing because we don't always install python3-argcomplete on all of our platforms. However, I think that is just wrong, since that is the experience we want to give to users. There are 2 fixes here:

  1. Add a dependency on python3-argcomplete to this package.
  2. Update our CI scripts to install python3-argcomplete as necessary.

I'll fix up this PR with 1, and open a separate PR for 2.

@clalancette
Add in appropriate dependency on python3-argcomplete.
fc583b2 
Also do a minor refactoring of the dependencies while we
are here.

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
@clalancette
Fix for Windows.
2564a21 
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
@fujitatomoya
Copy link

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@fujitatomoya
Copy link

Ah, sorry CI is not ready yet? if so, previous CI is expected to fail again.

@clalancette
Copy link
Contributor Author

Ah, sorry CI is not ready yet? if so, previous CI is expected to fail again.

That's right, it wasn't ready yet. But it now is. I ran CI for this over in ros2/ci#746 (comment) , and it was happy. So going ahead and merging this one now.

@clalancette clalancette merged commit 73f1b0d into rolling Feb 1, 2024
3 checks passed
@clalancette clalancette deleted the clalancette/fix-warnings branch February 1, 2024 13:10
@mikaelarguedas mikaelarguedas mentioned this pull request Apr 27, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahcorde ahcorde ahcorde approved these changes

@fujitatomoya fujitatomoya fujitatomoya approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@clalancette @fujitatomoya @ahcorde