Skip to content
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

Android native logs #8384

Merged
merged 3 commits into from
Dec 17, 2024
Merged

Android native logs #8384

merged 3 commits into from
Dec 17, 2024

Conversation

larkox
Copy link
Contributor

@larkox larkox commented Nov 28, 2024

Summary

Add android Native Logs. IOS logs will be added in a later PR.

Related PR

mattermost/react-native-turbo-log#6

Ticket Link

Part of https://mattermost.atlassian.net/browse/MM-61970

Release Note

Android: App logs will show information from the native side

@larkox larkox added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Nov 28, 2024
@larkox larkox requested a review from enahum November 28, 2024 16:14
@larkox larkox added the Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) label Nov 28, 2024
Copy link
Contributor

@enahum enahum 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, some changes needed in the library and then update this once the library npm package is released

Copy link
Contributor

Choose a reason for hiding this comment

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

before merging this PR, we should update the library and remove this patch

@larkox larkox removed the Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) label Dec 10, 2024
@@ -24,7 +24,7 @@
"@mattermost/react-native-emm": "1.5.0",
"@mattermost/react-native-network-client": "1.7.3",
"@mattermost/react-native-paste-input": "0.8.0",
"@mattermost/react-native-turbo-log": "0.4.0",
"@mattermost/react-native-turbo-log": "github:mattermost/react-native-turbo-log#d8ddf5e7974546aff3e83b2c563907eb609ac5f4",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@enahum Let's use the github commit until we have the iOS version in place and we bump the version of the library.

@larkox
Copy link
Contributor Author

larkox commented Dec 10, 2024

@yasserfaraazkhan On a dev build, by defualt, the react native part won't create logs, so test this with a production build.

The idea is that now you will have new logs, based on what happens on the native side (specially, when receiving notifications). Also, make sure that iOS has no relevant changes (no sudden crashes and the logs work just fine (just not the native part)).

@yasserfaraazkhan yasserfaraazkhan added the Build App for Android Build the mobile app for Android label Dec 10, 2024
Copy link
Contributor

@yasserfaraazkhan yasserfaraazkhan left a comment

Choose a reason for hiding this comment

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

verified we are getting new logs

image

@larkox larkox added QA Review Done and removed 3: QA Review Requires review by a QA tester labels Dec 12, 2024
@enahum
Copy link
Contributor

enahum commented Dec 17, 2024

going to merge this.

@enahum enahum merged commit 640ff93 into main Dec 17, 2024
20 checks passed
@enahum enahum deleted the nativeLogs branch December 17, 2024 10:16
@amyblais amyblais added this to the v2.25.0 milestone Dec 17, 2024
@amyblais amyblais added the Docs/Needed Requires documentation label Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2: Dev Review Requires review by a core commiter Build App for Android Build the mobile app for Android Docs/Needed Requires documentation QA Review Done release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants