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

refactor(ui): coroutine based user status retrieval. #13927

Merged
merged 2 commits into from
Nov 7, 2024

Conversation

ardevd
Copy link
Collaborator

@ardevd ardevd commented Nov 1, 2024

Replaced ancient async task with a coroutine based alternative. This comes with numerous advantages.

  • Improved performance due to the nature of coroutines and the reduced overhead associated with AsyncTasks and threads. Approx a %50 performance increase was measured during testing.
  • Passing references to fragment lifecycle context to an AsyncTask is generally a bad idea.
  • Improved decoupling of UI and backend logic.
  • Less boilerplate code.

/*
* Nextcloud - Android Client
*
* SPDX-FileCopyrightText: 2024 Your Name <[email protected]>
Copy link
Member

Choose a reason for hiding this comment

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

You might want to update this line from the template ;)

@AndyScherzinger
Copy link
Member

Hi @ardevd - long time no see 😃
Hope you have been good!

PR looks good to me, just one minor thing regarding the template license header, else 👍

@ardevd
Copy link
Collaborator Author

ardevd commented Nov 1, 2024

@AndyScherzinger glad to see you're still around :) Fixed the license header now, I think.

Replaced ancient async task with a coroutine based alternative.
This comes with numerous advantages.

- Improved performance due to the nature of coroutines and the reduced overhead associated with AsyncTasks and threads.
  Approx a %50 performance increase was measured during testing.
- Passing references to fragment lifecycle context to an AsyncTask is generally a bad idea.
@AndyScherzinger
Copy link
Member

Yes I am - Also happy to keep being around while I don't have much time these days for coding work :/
Hence looped in Tobias and Alper

@tobiasKaminsky
Copy link
Member

Welcome back 🎉

Can you check "spotlessKotlinCheck" and "Codacy"? There is something to do.
And sign off commits, for DCO?

Rest is false positive.

Copy link
Collaborator

@alperozturk96 alperozturk96 left a comment

Choose a reason for hiding this comment

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

Tested working. Thank you 💯

codacy

Signed-off-by: tobiasKaminsky <[email protected]>
Copy link

github-actions bot commented Nov 5, 2024

Codacy

Lint

TypemasterPR
Warnings5959
Errors33

SpotBugs

@tobiasKaminsky tobiasKaminsky merged commit 6d0aaa6 into master Nov 7, 2024
19 of 21 checks passed
@tobiasKaminsky tobiasKaminsky deleted the replace-status-async-task branch November 7, 2024 06:38
@AndyScherzinger AndyScherzinger added this to the Nextcloud App 3.31.0 milestone Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants