Conversation
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
.../org/wordpress/android/ui/jetpack/backup/download/usecases/GetBackupDownloadStatusUseCase.kt
Outdated
Show resolved
Hide resolved
|
You can test the changes on this Pull Request by downloading the APK here. |
ParaskP7
left a comment
There was a problem hiding this comment.
👋 @zwarm !
I am approving this PR, I bet it wasn't fun on your part to go through all these for such a simple change. 😅
Since I don't have the full context here, I am going to let you merge that to develop when you know it is the right time (don't want to break anything myself).
| } | ||
| else -> { | ||
| AppLog.e(T.JETPACK_RESTORE, "Error initializing ${this.javaClass.simpleName}") | ||
| AppLog.e(T.JETPACK_REWIND, "Error initializing ${this.javaClass.simpleName}") |
There was a problem hiding this comment.
Question (❓): Why did this change to REWIND? Since rewind is used in general with backup download as well, as a term that is (like rewindId), I think it is better if we use RESTORE as a naming convention on restore specific cases. I try to do that everywhere, when refactoring field, parameters, functions, etc. Thus, I asking this question to understand if I am missing any information in regards to that.
There was a problem hiding this comment.
Why did this change to REWIND?
Because I chose RESTORE before I realized that the value was already defined in Utils. I used the value that was preexisting.
Parent #13328 & #13329
This PR logs an error when:
Please let me know if you feel this code is overkill.
Notes
I was too optimistic with this PR. The AppLog is in WordPress.Utils and I not realize that there is already a PR out there that updated values, but that PR hasn't been merged yet. I have a request out to figure out how to get that pushed, then I will come back and update this one with the tag names.
To test:
PR submission checklist:
RELEASE-NOTES.txtif necessary.