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 Sample App Consolidation and Kotlin Conversion #410

Merged
merged 16 commits into from
Aug 12, 2022

Conversation

handstandsam
Copy link
Contributor

@handstandsam handstandsam commented Aug 11, 2022

It is confusing having two sample Android Applications. This is a step in the right direction to move everything to Kotlin and cleanup the codebase. There will still be more iterations that would make this ideal, but it's a good step forward.

Completed:

  • Removed viewBinding
  • Convert Original Sample App to Kotlin
  • Use local.properties method of setting the APP_KEY for the original sample app
  • Move over the code from the new sample app into the Android sample app.
  • Migrated AsyncTasks to Kotlin Coroutines
  • Fixed layout on smaller screens
Screen Shot 2022-08-12 at 2 45 20 PM

@handstandsam handstandsam force-pushed the handstandsam/android-consolidation-and-kotlin branch from e0efad2 to 892f417 Compare August 12, 2022 18:42
Copy link
Contributor

@rharter rharter 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 reasonable to me.

Not a blocker, but I think the example could be easier for users to use if everything wasn't named Dropbox*, since that makes it really hard to peruse and understand what is part of the Dropbox API and what's part of the example (i.e. DropboxApi isn't actually part of the Dropbox SDK 😆 ). Perhaps making the sample app Sample or something could help.

@handstandsam handstandsam force-pushed the handstandsam/android-consolidation-and-kotlin branch 5 times, most recently from 9f2aa69 to 45bb8fe Compare August 12, 2022 20:19
Copy link
Contributor

@alisonthemonster alisonthemonster left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thank you!

@handstandsam handstandsam force-pushed the handstandsam/android-consolidation-and-kotlin branch from 45bb8fe to 7d1f750 Compare August 12, 2022 20:20
@handstandsam handstandsam force-pushed the handstandsam/android-consolidation-and-kotlin branch from 7d1f750 to 21f0d76 Compare August 12, 2022 20:25
@handstandsam
Copy link
Contributor Author

Good call @rharter . I updated the naming to be clear. Also I moved all the *Task.kt (async task leftovers) to the DropboxApiWrapper.kt file as a metho
Screen Shot 2022-08-12 at 4 58 08 PM
d. 😄

@handstandsam handstandsam merged commit 97f2c2b into master Aug 12, 2022
@handstandsam handstandsam deleted the handstandsam/android-consolidation-and-kotlin branch September 22, 2022 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants