Skip to content

Conversation

@fujitatomoya
Copy link
Collaborator

close #279

Signed-off-by: Tomoya.Fujita [email protected]

@fujitatomoya
Copy link
Collaborator Author

@iuhilnehc-ynos @Ada-King
CC: @clalancette

either of you, could you take a look at this? it seems that ros2/rcl#741 does not require this anymore, but i guess that this would be useful.

Copy link
Collaborator

@iuhilnehc-ynos iuhilnehc-ynos left a comment

Choose a reason for hiding this comment

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

@fujitatomoya

nitpick, just some minor comments.

@fujitatomoya fujitatomoya force-pushed the feature-20200823-case-insensitive-cmp branch from 2f949db to 5a6a26c Compare August 28, 2020 07:47
@fujitatomoya
Copy link
Collaborator Author

@iuhilnehc-ynos @Ada-King

comments are addressed, could you check when you got time?

@fujitatomoya fujitatomoya force-pushed the feature-20200823-case-insensitive-cmp branch from 66e0cb3 to 3a2e1d4 Compare September 2, 2020 02:10
@fujitatomoya
Copy link
Collaborator Author

@clalancette @iuhilnehc-ynos @Ada-King

could you do review again?

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.

A couple more things to fix, then this will be ready to have CI.

@fujitatomoya
Copy link
Collaborator Author

@clalancette

thanks for the suggestion, i changed it back along with strcasecmp behavior. could you check when you got time?

@clalancette
Copy link
Contributor

clalancette commented Sep 3, 2020

CI (only up to rcutils since this is a new API):

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

@clalancette
Copy link
Contributor

@fujitatomoya It looks like Windows has some warnings with this change. Can you look into it? Thanks.

@fujitatomoya
Copy link
Collaborator Author

@clalancette

got it, i will address them in today. will get back to you.

@fujitatomoya
Copy link
Collaborator Author

@clalancette

i believe windows is now comfortable with 9b586f2. Could you run CI again? thank!

@clalancette
Copy link
Contributor

Thanks. Here's another CI run:

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

@clalancette
Copy link
Contributor

Looks good now, thanks for iterating @fujitatomoya !

@clalancette clalancette merged commit a935f27 into ros2:master Sep 4, 2020
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.

Case insensitive string compare function.

4 participants