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

Implement rcutils_mkdir. #166

Merged
merged 4 commits into from
Sep 30, 2019
Merged

Implement rcutils_mkdir. #166

merged 4 commits into from
Sep 30, 2019

Conversation

clalancette
Copy link
Contributor

@clalancette clalancette commented Jun 28, 2019 •
edited
Loading

Needed for the spdlog logging backend.

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

Connected to ros2/rcl_logging#17

@clalancette clalancette mentioned this pull request Jun 28, 2019
Merged
mjcarroll
mjcarroll requested changes Jul 9, 2019
View reviewed changes
Copy link
Member

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

Can you add some tests for this?

@ghost
Copy link

ghost commented Jul 31, 2019

(going through ci tutorial, test launch ci)

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

@clalancette
Implement rcutils_mkdir.
735cddf 
Needed for the spdlog logging backend.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Add in tests for rcutils_mkdir.
8075e9d 
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette clalancette mentioned this pull request Sep 27, 2019
Closed
8 tasks
@clalancette
Copy link
Contributor Author

Can you add some tests for this?

Done in 8075e9d

@clalancette
Copy link
Contributor Author

New CI is in the meta-issue: ros2/ros2#789 (comment)

mjcarroll
mjcarroll approved these changes Sep 27, 2019
View reviewed changes
Copy link
Member

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

LGTM with green CI and minor documentation question.

jacobperron
jacobperron approved these changes Sep 30, 2019
View reviewed changes
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

LGTM one nit

include/rcutils/filesystem.h Outdated Show resolved Hide resolved
@clalancette
Fix small nit for documentation.
412dafb 
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Add in a warning that the thread isn't thread-safe.
3121926 
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Copy link
Contributor Author

All right, merging this one in as a pre-requisite to the spdlog logging framework. Thanks for the reviews.

@clalancette clalancette merged commit 8983a69 into master Sep 30, 2019
@delete-merged-branch delete-merged-branch bot deleted the spdlog branch September 30, 2019 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mjcarroll mjcarroll mjcarroll approved these changes

@jacobperron jacobperron jacobperron 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.
3 participants
@clalancette @mjcarroll @jacobperron