-
Notifications
You must be signed in to change notification settings - Fork 24.6k
Migrate ViewGroupManager to kotlin #50895
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.
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
Conversation
@mateoguzmana @cortinico @mdvacca Can you review this |
@mateoguzmana are you doing a first pass? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Noticed a few things
...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.kt
Outdated
Show resolved
Hide resolved
...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.kt
Show resolved
Hide resolved
...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.kt
Outdated
Show resolved
Hide resolved
...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.kt
Show resolved
Hide resolved
...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.kt
Show resolved
Hide resolved
@mateoguzmana Thanks for the review . I have resolved the comments . Can you re check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more thing, the build seems failing as well, could you check that?
...es/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewGroupManager.txt
Outdated
Show resolved
Hide resolved
|
@cortinico Can you please review this? |
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@cortinico Can we merge this 😅 |
Nope because this is breaking a bunch of things internally. I will have to look into this and change it so that the internal builds are not broken |
@cortinico merged this pull request in 761b158. |
This pull request was successfully merged by @riteshshukla04 in 761b158 When will my fix make it into a release? | How to file a pick request? |
Thanks for merging @cortinico @mateoguzmana . If you have more files to migrate, please let me know . I enjoy contributing here |
Summary:
This PR aims to migrate ViewGroupManager to kotlin as part of #50513
Changelog:
[ANDROID][CHANGED]Migrate ViewGroupManager to kotlin
Test Plan:
Tested with RN tester with old and new arch