-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Convert notes adapter to kotlin #20129
Convert notes adapter to kotlin #20129
Conversation
This appears to be dead code.
This is a preparation for converting to Kotlin.
Manual changes are intentionally ommited from this commit in order to better track the rest of the conversion / refactor in later steps.
We need to mark this to avoid warnings as errors, but we will be refactoring the deprecated usages in later stages.
This was a static helper method, and is now a companion function.
Generated by 🚫 dangerJS |
Generated by 🚫 Danger |
📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
|
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.
Nice work! Thanks for converting the adapter to Kotlin.
The test result is as expected.
There's no blocker, but there are some details can be refactored.
Feel free to merge this PR.
WordPress/src/main/java/org/wordpress/android/ui/notifications/adapters/NotesAdapter.kt
Outdated
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/ui/notifications/adapters/NotesAdapter.kt
Outdated
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/ui/notifications/adapters/NotesAdapter.kt
Outdated
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/ui/notifications/adapters/NotesAdapter.kt
Outdated
Show resolved
Hide resolved
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.
Thank you for grabbing the opportunity to improve the codebase @mkevins 🙇
The code looks great and there was no change in the app behaviour in my tests 🎉
Thank you both for reviewing and testing! |
Description
This PR refactors the NotesAdapter class to Kotlin. This is intended to improve the further development of the features we are adding in the notifications refresh project.
To Test:
Regression Notes
Notifications
Manual tests
With the code in its current state, adding automated tests would exceed the scope allotted for this work.
PR Submission Checklist:
RELEASE-NOTES.txt
if necessary.Testing Checklist: