Skip to content

Switch to using the filesystem implementation in rcpputils.#239

Merged
clalancette merged 1 commit intofoxy-develfrom
clalancette/switch-to-rcpputils-filesystem
Nov 2, 2020
Merged

Switch to using the filesystem implementation in rcpputils.#239
clalancette merged 1 commit intofoxy-develfrom
clalancette/switch-to-rcpputils-filesystem

Conversation

@clalancette
Copy link
Copy Markdown
Contributor

That implementation is more complete and better tested. This
also gets us closer to remove the pluginlib implementation
of the filesystem helper.

Note that this doesn't really change the dependencies of this
package; this package already recursively depended on rcpputils,
now it just directly depends on it.

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

That implementation is more complete and better tested.  This
also gets us closer to remove the pluginlib implementation
of the filesystem helper.

Note that this doesn't really change the dependencies of this
package; this package already recursively depended on rcpputils,
now it just directly depends on it.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Copy link
Copy Markdown
Contributor Author

I'm going to run CI on this, but as far as I can tell there are no tests that exercise this piece of code. So CI will only tell us if it builds.

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

Copy link
Copy Markdown
Member

@cottsay cottsay left a comment

Choose a reason for hiding this comment

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

Seems fine to me.

@clalancette
Copy link
Copy Markdown
Contributor Author

I did some basic testing of rqt with this patch in place, and I didn't see any changes. With the green CI and the approval, I'm going to go ahead and merge it. Thanks for the feedback.

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.

2 participants