Skip to content

[cxx-interop] Add snake_case to CXXMethodBridging #41905

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
Merged
merged 2 commits into from
May 17, 2022
Merged

[cxx-interop] Add snake_case to CXXMethodBridging #41905

merged 2 commits into from
May 17, 2022

Conversation

cabmeurer
Copy link
Contributor

@cabmeurer cabmeurer commented Mar 19, 2022 •
edited
Loading

Update CxxMethodBridging to transform snake_case names.
Also fixed some typos unkown -> unknown

related: #40842

@bro4all

@cabmeurer cabmeurer mentioned this pull request Mar 19, 2022
Closed
@cabmeurer
Copy link
Contributor Author

@zoecarver @hyp Everything is working correctly on my local machine 🙌

@zoecarver
Copy link
Contributor

@swift-ci please test.

@cabmeurer
Copy link
Contributor Author

@zoecarver is this good to merge?

zoecarver
zoecarver reviewed Mar 26, 2022
View reviewed changes
@zoecarver
Copy link
Contributor

Sorry a few more comments.

@cabmeurer cabmeurer force-pushed the cabmeurer/add-snake_case-to-CXXMethodBridging branch from 0d5f9bc to a3adf3a Compare March 29, 2022 00:38
@cabmeurer cabmeurer requested a review from zoecarver March 29, 2022 15:20
cabmeurer
cabmeurer commented Mar 29, 2022
View reviewed changes
cabmeurer
cabmeurer commented Mar 29, 2022
View reviewed changes
@cabmeurer cabmeurer force-pushed the cabmeurer/add-snake_case-to-CXXMethodBridging branch 2 times, most recently from cac8310 to d218daa Compare March 30, 2022 00:49
@cabmeurer
Copy link
Contributor Author

cabmeurer commented Mar 30, 2022 •
edited
Loading

@zoecarver i made the suggested changes discussed today

@cabmeurer cabmeurer force-pushed the cabmeurer/add-snake_case-to-CXXMethodBridging branch 6 times, most recently from a1d1b8f to 5f0e93b Compare March 31, 2022 05:51
@cabmeurer
Copy link
Contributor Author

@bro4all could you review this when you get a chance?

@cabmeurer cabmeurer force-pushed the cabmeurer/add-snake_case-to-CXXMethodBridging branch from 5f0e93b to 8945a25 Compare April 2, 2022 03:31
zoecarver
zoecarver reviewed Apr 4, 2022
View reviewed changes
zoecarver
zoecarver reviewed Apr 4, 2022
View reviewed changes
zoecarver
zoecarver approved these changes Apr 4, 2022
View reviewed changes
Copy link
Contributor

@zoecarver zoecarver 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. Thanks for your patience.

@zoecarver
Copy link
Contributor

@swift-ci please test.

@zoecarver
Copy link
Contributor

@swift-ci please test windows.

@cabmeurer cabmeurer force-pushed the cabmeurer/add-snake_case-to-CXXMethodBridging branch from 8945a25 to f1a8e6d Compare April 4, 2022 16:28
@cabmeurer
Copy link
Contributor Author

cabmeurer commented Apr 4, 2022 •
edited
Loading

This looks good to me. Thanks for your patience.

No worries! Thanks for checking everything, I think CI should pass now

@cabmeurer
Copy link
Contributor Author

@zoecarver could you run CI for me when you get a chance?

@zoecarver
Copy link
Contributor

@swift-ci please test.

zoecarver
zoecarver reviewed Apr 25, 2022
View reviewed changes
@cabmeurer cabmeurer force-pushed the cabmeurer/add-snake_case-to-CXXMethodBridging branch from 4146005 to 945dcad Compare April 26, 2022 20:41
@cabmeurer
Copy link
Contributor Author

@zoecarver i think this should work now

zoecarver
zoecarver reviewed Apr 27, 2022
View reviewed changes
@cabmeurer cabmeurer force-pushed the cabmeurer/add-snake_case-to-CXXMethodBridging branch from 945dcad to 9d35eb6 Compare April 28, 2022 22:09
@cabmeurer
Copy link
Contributor Author

@zoecarver could you look this over and run CI when you get a chance?

@cabmeurer
Copy link
Contributor Author

@xwu could you run CI for me when you get a chance? Thank you!

@xwu
Copy link
Collaborator

xwu commented May 3, 2022

@swift-ci test

cabmeurer
cabmeurer commented May 3, 2022
View reviewed changes
@cabmeurer cabmeurer force-pushed the cabmeurer/add-snake_case-to-CXXMethodBridging branch from 9d35eb6 to 34dad65 Compare May 9, 2022 23:05
@zoecarver
Copy link
Contributor

@swift-ci please test

xwu
xwu reviewed May 10, 2022
View reviewed changes
@cabmeurer cabmeurer force-pushed the cabmeurer/add-snake_case-to-CXXMethodBridging branch from 34dad65 to 15eb0e4 Compare May 10, 2022 23:46
@cabmeurer
Copy link
Contributor Author

@xwu could you run CI for me when you get a chance?

@xwu
Copy link
Collaborator

xwu commented May 10, 2022

@swift-ci test

@cabmeurer cabmeurer force-pushed the cabmeurer/add-snake_case-to-CXXMethodBridging branch from 15eb0e4 to 1eeda42 Compare May 11, 2022 19:58
@hyp
Copy link
Contributor

hyp commented May 11, 2022

@swift-ci please test

zoecarver
zoecarver reviewed May 12, 2022
View reviewed changes
@cabmeurer cabmeurer force-pushed the cabmeurer/add-snake_case-to-CXXMethodBridging branch from 1eeda42 to a7fc073 Compare May 13, 2022 02:36
@zoecarver
Copy link
Contributor

@swift-ci please test.

@cabmeurer
Copy link
Contributor Author

@zoecarver I think this should be good to go!

zoecarver
zoecarver reviewed May 16, 2022
View reviewed changes
@cabmeurer cabmeurer force-pushed the cabmeurer/add-snake_case-to-CXXMethodBridging branch from a7fc073 to 7d50071 Compare May 16, 2022 22:47
@zoecarver
Copy link
Contributor

@swift-ci please test and merge.

@zoecarver
Copy link
Contributor

@swift-ci please test Windows

@zoecarver
Copy link
Contributor

Thank you!

@swift-ci swift-ci merged commit 415c5c1 into swiftlang:main May 17, 2022
@cabmeurer cabmeurer deleted the cabmeurer/add-snake_case-to-CXXMethodBridging branch May 17, 2022 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xwu xwu xwu left review comments

@zoecarver zoecarver zoecarver approved these changes

Assignees
No one assigned
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@cabmeurer @zoecarver @xwu @WowbaggersLiquidLunch @hyp @swift-ci