Repository navigation
feat: replace logging by tmc component - #11668
Merged
wmontwe merged 10 commits intoOct 14, 2026
Merged
Conversation
github-actions
Bot
added
the
tb-team
Contributor
|
✅ Validation Passed: All report and feature-flag labels are correctly set. |
wmontwe
added
the
report: exclude
wmontwe
force-pushed
the
feat-replace-logging-by-tmc-component
branch
from
October 12, 2026 14:29
5283b99 to
bd5c2c3
Compare
wmontwe
marked this pull request as ready for review
October 12, 2026 14:29
thunderbird-botmobile
Bot
added
the
pr-sentinel: ready for review
wmontwe
force-pushed
the
feat-replace-logging-by-tmc-component
branch
from
October 12, 2026 15:13
bd5c2c3 to
27958ea
Compare
wmontwe
force-pushed
the
feat-replace-logging-by-tmc-component
branch
from
October 12, 2026 15:16
27958ea to
013da00
Compare
wmontwe
force-pushed
the
feat-replace-logging-by-tmc-component
branch
from
October 12, 2026 15:27
013da00 to
60e2518
Compare
wmontwe
force-pushed
the
feat-replace-logging-by-tmc-component
branch
from
October 13, 2026 13:28
60e2518 to
4d37158
Compare
rafaeltonholo
left a comment
rafaeltonholo
left a comment
Member
There was a problem hiding this comment.
I have just a small concern about the DefaultStorageMigrationHelper; other than that all good to me.
| } | ||
|
|
||
| private fun syncDebugLogFile(directoryProvider: DirectoryProvider): PlatformFile { | ||
| val filesDirectory = requireNotNull(directoryProvider.getFilesDir().path) |
Member
There was a problem hiding this comment.
nit: I would add a message explaining why the path is required to be null, so it makes easier to debug in case of any bugs.
Member
Author
There was a problem hiding this comment.
I added a description that the path is required to be not null.
| class DefaultStorageMigrationHelper : StorageMigrationHelper { | ||
| override fun readAllValues(db: SQLiteDatabase): Map<String, String> { | ||
| return db.query(TABLE_NAME, arrayOf(KEY_COLUMN, VALUE_COLUMN), null, null, null, null, null).use { | ||
| it.map { cursor -> | ||
| val key = cursor.getStringOrThrow(KEY_COLUMN) | ||
| val value = cursor.getStringOrThrow(VALUE_COLUMN) | ||
| Log.d("Loading key '%s', value = '%s'", key, value) | ||
| Log.d(TAG, "Loading key '%s', value = '%s'", key, value) |
Member
There was a problem hiding this comment.
This log will potentially expose sensitive data. I would recommend removing it or wrap it inside an if checking whetherBuildConfig.DEBUG is enabled.
I wouldn't let that be logged even if users have enabled the Debug Logs in the production app.
Same for the other logs in this class that logs the value.
Member
Author
There was a problem hiding this comment.
I removed the values from the logging.
wmontwe
force-pushed
the
feat-replace-logging-by-tmc-component
branch
from
October 14, 2026 09:53
7f8144c to
1007976
Compare
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contribution Summary
Linked Issue/Ticket: Resolves #11716
Depends on: thunderbird/thunderbird-mobile-components#173 being merged and released
Description
Replace the app’s logging implementation with the logger from TMC. Migrate callers and tests, update debug-log export and documentation, and remove unused logging code.
Expected to fail until the library is released.
AI Disclosure
Select one of the following (mandatory)