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(mobile): entities and models #9182

Merged
merged 6 commits into from
May 1, 2024

Conversation

alextran1502
Copy link
Contributor

@alextran1502 alextran1502 commented Apr 30, 2024

  • Moved all Isar models to a directory called entities
  • Moved all models across the app to a directory called models

Copy link

cloudflare-pages bot commented Apr 30, 2024

Deploying immich with  Cloudflare Pages  Cloudflare Pages

Latest commit: 744fc7d
Status: ✅  Deploy successful!
Preview URL: https://c9b228b0.immich.pages.dev
Branch Preview URL: https://chore-mobile-refactor-entiti.immich.pages.dev

View logs

@avidal
Copy link

avidal commented Apr 30, 2024

Bit of a drive-by, but should this include a change to the model_template and corresponding block in the README?

@alextran1502
Copy link
Contributor Author

@avidal Makes sense, we are still working on restructuring that so potentially that template might be gone as well

Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

In the server we got rid of the domain folder. Having something similar in mobile might be nice as well. Maybe just folders for entities, services, models, interfaces, and repositories would be better.

Copy link
Contributor

@martyfuhry martyfuhry left a comment

Choose a reason for hiding this comment

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

  • asset_viewer/image_providers could be a top-level module.
  • home/ui/asset_grid could just be a top-level module of asset_grid.
  • I'm not a fan of the shared module, but it was there before so this doesn't need to address that now. I only bring it up in case others have better ideas what to do with it.
  • ui and view sort of mean the same thing to me when I look at the modules. It seems like ui is made up of composible Widget components while view is made up of pages of the application which compose from the ui. Maybe we can rename view to page?

Thank you for making this massive improvement! It was long overdue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this can live in backup/services

@fyfrey
Copy link
Contributor

fyfrey commented Apr 30, 2024

Looking forward to the restructured code base!

I agree with @jrasm91, instead of another level of nesting (domain), let us directly have folders for entities, models, ... one level higher up.

Also agree with @martyfuhry, but we can do these changes in another PR.

@jrasm91
Copy link
Contributor

jrasm91 commented May 1, 2024

A lot of web projects use a routes folder which basically corresponds to application pages. It could make sense to have a folder for top level pages, organized that same way, and then have sub components (like buttons, spinners, asset grid, thumbnail, etc.) in a separate folder.

@alextran1502 alextran1502 merged commit f057fe0 into main May 1, 2024
24 checks passed
@alextran1502 alextran1502 deleted the chore/mobile-refactor-entities branch May 1, 2024 02:36
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.

None yet

5 participants