Skip to content

Updated rcpputils path API#2579

Merged
ahcorde merged 6 commits intorollingfrom
ahcorde/rolling/update_path_api
Jul 24, 2024
Merged

Updated rcpputils path API#2579
ahcorde merged 6 commits intorollingfrom
ahcorde/rolling/update_path_api

Conversation

@ahcorde
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde commented Jul 8, 2024

Related PR ros2/rcpputils#196

Updated deprecated API

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from clalancette July 8, 2024 18:51
@ahcorde ahcorde self-assigned this Jul 8, 2024
@christophebedard
Copy link
Copy Markdown
Member

There's another rcpputils::fs that can be changed to std::filesystem here:

namespace fs = rcpputils::fs;

@christophebedard
Copy link
Copy Markdown
Member

Here too:

rcpputils::fs::path test_resources_path{TEST_RESOURCES_DIRECTORY};

@christophebedard
Copy link
Copy Markdown
Member

Another:

rcpputils::fs::path test_resources_path{TEST_RESOURCES_DIRECTORY};

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from christophebedard July 11, 2024 10:26
ahcorde added 2 commits July 12, 2024 12:27
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
@ahcorde
Copy link
Copy Markdown
Contributor Author

ahcorde commented Jul 12, 2024

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

@ahcorde ahcorde requested a review from christophebedard July 12, 2024 10:40
Copy link
Copy Markdown
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.

One more thing I think we should change.

@ahcorde ahcorde requested a review from clalancette July 12, 2024 12:22
Copy link
Copy Markdown
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.

This looks good to me with green CI (it doesn't even technically need to wait for ros2/rcpputils#196 to be merged).

Copy link
Copy Markdown
Member

@christophebedard christophebedard left a comment

Choose a reason for hiding this comment

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

LGTM!

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Copy link
Copy Markdown
Member

@christophebedard christophebedard left a comment

Choose a reason for hiding this comment

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

Looks good to me with green CI.

Copy link
Copy Markdown
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.

Looks good to me with green CI.

@ahcorde
Copy link
Copy Markdown
Contributor Author

ahcorde commented Jul 23, 2024

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

@ahcorde ahcorde merged commit b1fdb18 into rolling Jul 24, 2024
@ahcorde ahcorde deleted the ahcorde/rolling/update_path_api branch July 24, 2024 08:18
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.

3 participants