Skip to content

Conversation

@oddlyspaced
Copy link
Contributor

Summary:

Saw a lot of PRs and push to convert existing Java code to Kotlin, and wanted to contribute to the cause. This is a starting point for me so I can understand more about the Java to Kotlin conversion process for ReactAndroid package.

Changelog:

  • Converted AndroidInfoHelpers to Kotlin class
  • Converted AndroidInfoModule to Kotlin object and updated usage of it across the package

Pick one each for the category and type tags:

[ANDROID] [CHANGED] - Migrated systeminfo module code from Java to Kotlin

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests

Test Plan:

Verified build on local dev environment.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 14, 2024
@oddlyspaced oddlyspaced changed the title chore: convert systeminfo module classes to kotlin chore(Android): convert systeminfo module classes to kotlin Nov 14, 2024
@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Nov 14, 2024
@oddlyspaced
Copy link
Contributor Author

Had to suppress the deprecation for SERIAL param in Android Info Module to get the package to compile. However in the Android docs it mentions now that the field is set to #UNKNOWN. Should I replace the Serial field with Unknown ?

https://developer.android.com/reference/android/os/Build#SERIAL

Copy link
Member

@javache javache left a comment

Choose a reason for hiding this comment

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

Thanks for contributing here! We actually have a lot of tooling internally to help us do these migrations automatically, but we're doing them one module at a time, since it's easy for regressions to sneak in. This seems pretty safe though.

@oddlyspaced
Copy link
Contributor Author

@javache I've added JvmStatic annotation to all the public methods of the object. Let me know if something else needs to be done from my end 👍🏾

@oddlyspaced oddlyspaced requested a review from javache November 15, 2024 17:53
@oddlyspaced
Copy link
Contributor Author

@javache do let me know if something is remaining from my end 🙏

@oddlyspaced
Copy link
Contributor Author

@javache sorry for the tag again, but anything that needs to be done by me ?

@oddlyspaced oddlyspaced requested a review from javache November 20, 2024 18:34
@facebook-github-bot
Copy link
Contributor

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@oddlyspaced
Copy link
Contributor Author

oddlyspaced commented Nov 21, 2024

@javache Anything I need to do to fix these failing tests? 😓

@javache
Copy link
Member

javache commented Nov 21, 2024

@javache Anything I need to do to fix these failing tests? 😓

Nope, I'm working on some internal build tooling but should be merged soon.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Nov 22, 2024
@facebook-github-bot
Copy link
Contributor

@javache merged this pull request in 8dc2c90.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @oddlyspaced in 8dc2c90

When will my fix make it into a release? | How to file a pick request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants