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

Extract image loader code & encapsulate logic #127

Closed
wants to merge 1 commit into from

Conversation

hieuwu
Copy link
Collaborator

@hieuwu hieuwu commented Nov 9, 2024

  • Move functions that handle image into editManager
  • Extract logic of loading image & to be out of ViewModel. The BitmapLoader contains logic of loading image from path, uri into EditManager & hides the logic of glide for bitmap loading

@hieuwu hieuwu self-assigned this Nov 9, 2024
@kirillt kirillt changed the base branch from main to develop November 9, 2024 14:35
@kirillt
Copy link
Member

kirillt commented Nov 9, 2024

@shubertm could you check how difficult are the conflicts?

@hieuwu we have big stale PR from develop to main, I guess if we merge yours into develop it should be safer. But if you prefer, feel free changing base back to main. We want to get rid of develop for now.

@hieuwu
Copy link
Collaborator Author

hieuwu commented Nov 9, 2024

@shubertm could you check how difficult are the conflicts?

@hieuwu we have big stale PR from develop to main, I guess if we merge yours into develop it should be safer. But if you prefer, feel free changing base back to main. We want to get rid of develop for now.

Let's merge into develop then. We could prefer merging that big PR first. I can handle this later

@shubertm
Copy link
Member

@shubertm could you check how difficult are the conflicts?
@hieuwu we have big stale PR from develop to main, I guess if we merge yours into develop it should be safer. But if you prefer, feel free changing base back to main. We want to get rid of develop for now.

Let's merge into develop then. We could prefer merging that big PR first. I can handle this later

Yes, we should merge the PR #126 first, changes here are small. Could be resolved with main later. And because the file structure was also refactored.

@shubertm
Copy link
Member

Good work @hieuwu 🚀 I have looked at the code, looks good. Let's minimize EditViewModel and EditManager more. I will approve once all conflicts are resolved

Copy link
Member

@shubertm shubertm left a comment

Choose a reason for hiding this comment

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

Let's merge #126 and resolve conflicts first

@kirillt kirillt deleted the branch develop November 14, 2024 09:45
@kirillt kirillt closed this Nov 14, 2024
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