Skip to content

introduce rcutils_strcasecmp, case insensitive string compare.#280

Merged
clalancette merged 5 commits intoros2:masterfrom
fujitatomoya:feature-20200823-case-insensitive-cmp
Sep 4, 2020
Merged

introduce rcutils_strcasecmp, case insensitive string compare.#280
clalancette merged 5 commits intoros2:masterfrom
fujitatomoya:feature-20200823-case-insensitive-cmp

Conversation

@fujitatomoya
Copy link
Copy Markdown
Collaborator

@fujitatomoya fujitatomoya commented Aug 23, 2020

close #279

Signed-off-by: Tomoya.Fujita Tomoya.Fujita@sony.com

@fujitatomoya fujitatomoya mentioned this pull request Aug 23, 2020
Closed
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Aug 27, 2020

@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.

iuhilnehc-ynos
iuhilnehc-ynos reviewed Aug 27, 2020
View reviewed changes
Copy link
Copy Markdown
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.

Ada-King
Ada-King reviewed Aug 27, 2020
View reviewed changes
@fujitatomoya fujitatomoya force-pushed the feature-20200823-case-insensitive-cmp branch from 2f949db to 5a6a26c Compare August 28, 2020 07:47
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Aug 28, 2020

@iuhilnehc-ynos @Ada-King

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

iuhilnehc-ynos
iuhilnehc-ynos reviewed Aug 28, 2020
View reviewed changes
clalancette
clalancette requested changes Aug 31, 2020
View reviewed changes
@fujitatomoya
introduce rcutils_strcasecmp, case insensitive string compare.
35027fa 
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya
add rcutils_strncasecmp with test.
3a2e1d4 
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya fujitatomoya force-pushed the feature-20200823-case-insensitive-cmp branch from 66e0cb3 to 3a2e1d4 Compare September 2, 2020 02:10
@fujitatomoya
add comparison result value, update test to check if less or greater …
f07f00e 
…than zero.

Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Sep 2, 2020

@clalancette @iuhilnehc-ynos @Ada-King

could you do review again?

clalancette
clalancette requested changes Sep 2, 2020
View reviewed changes
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.

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

@fujitatomoya
always return 0 if method is successfully called.
20b9c98 
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Sep 3, 2020

@clalancette

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

@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Sep 3, 2020 •
edited
Loading

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
Copy Markdown
Contributor

clalancette commented Sep 3, 2020

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

@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Sep 3, 2020

@clalancette

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

@fujitatomoya
windows unhappy, use ISO C and C++ conformant name instead.
9b586f2 
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya
Copy link
Copy Markdown
Collaborator Author

fujitatomoya commented Sep 4, 2020

@clalancette

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

@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Sep 4, 2020

Thanks. Here's another CI run:

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

@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Sep 4, 2020

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

Reviewers

@clalancette clalancette clalancette approved these changes

@iuhilnehc-ynos iuhilnehc-ynos iuhilnehc-ynos approved these changes

+1 more reviewer

@Ada-King Ada-King Ada-King approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Case insensitive string compare function.

4 participants

@fujitatomoya @clalancette @iuhilnehc-ynos @Ada-King