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

[New arch] Remove file #3214

Merged
merged 20 commits into from
May 7, 2021
Merged

Conversation

abelgardep
Copy link
Contributor

Related Issues

Implements #2864

Library PR (if needed): owncloud/android-library#394


@abelgardep abelgardep self-assigned this Apr 29, 2021
@abelgardep abelgardep linked an issue Apr 29, 2021 that may be closed by this pull request
20 tasks
Base automatically changed from new_arch/download_with_worker to new_arch/synchronization April 30, 2021 11:48
@abelgardep abelgardep force-pushed the new_arch/synchronization branch from 4b4e969 to a30fea4 Compare April 30, 2021 12:16
@abelgardep abelgardep force-pushed the new_arch/remove_file branch from 60ab175 to bedaf3e Compare April 30, 2021 12:31
@abelgardep abelgardep marked this pull request as ready for review May 4, 2021 16:31
Copy link
Collaborator

@JuancaG05 JuancaG05 left a comment

Choose a reason for hiding this comment

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

I think the conversion from Java to Kotlin is ok, and the rest also looks fine. As in the android-library PR, let's wait for another review and its explicit approval 💯

if (file.isAvailableLocally) {
containsDown = true
}
// FIXME: 13/10/2020 : New_arch: Av.Offline
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe is better to use a TODO with a New arch tag so you can filter in the future what's pending? 🍻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I introduced fixmes when we migrated the database, so I can filter them to check what's missing to move to the new arch. The idea is basically the same as TODOS, so I'll keep it this way since I already started using fixmes. Anyway, I will consider your proposal for future refactors 👍

@abelgardep abelgardep force-pushed the new_arch/remove_file branch from 3ffae6b to 2686d8d Compare May 7, 2021 09:06
@abelgardep abelgardep merged commit 60904f9 into new_arch/synchronization May 7, 2021
@abelgardep abelgardep deleted the new_arch/remove_file branch May 7, 2021 10:00
@abelgardep abelgardep mentioned this pull request Jun 13, 2022
75 tasks
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.

[New Arch] Remove file/folder operation
3 participants