Skip to content

Move cache logic to own module#432

Merged
nicoburns merged 7 commits intoDioxusLabs:mainfrom
nicoburns:move-cache-logic-to-own-module
Apr 11, 2023
Merged

Move cache logic to own module#432
nicoburns merged 7 commits intoDioxusLabs:mainfrom
nicoburns:move-cache-logic-to-own-module

Conversation

@nicoburns
Copy link
Copy Markdown
Collaborator

@nicoburns nicoburns commented Apr 10, 2023

Objective

This is mostly a code quality improvement. It moves the caching logic to it's own module, and makes it object-oriented. A bonus is that it allows us to make the new Cache struct public which enables users implementing the LayoutTree trait manually to access our caching logic.

Context

Split out from #326.
Builds on top of, and is blocked by #431

Feedback wanted

  • General PR review
  • Is it a good idea to make the Cache struct public?

@nicoburns nicoburns added enhancement New feature or request code quality Make the code cleaner or prettier. blocked Cannot be advanced until something else changes labels Apr 10, 2023
@nicoburns nicoburns requested review from TimJentzsch and Weibye April 10, 2023 22:06
@nicoburns nicoburns force-pushed the move-cache-logic-to-own-module branch from e2c6d9b to e8d4280 Compare April 10, 2023 22:11
@nicoburns nicoburns mentioned this pull request Apr 10, 2023
13 tasks
Copy link
Copy Markdown
Collaborator

@TimJentzsch TimJentzsch left a comment

Choose a reason for hiding this comment

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

I don't have objections about making the caching pub

@nicoburns nicoburns merged commit 3bda644 into DioxusLabs:main Apr 11, 2023
@nicoburns nicoburns deleted the move-cache-logic-to-own-module branch April 11, 2023 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked Cannot be advanced until something else changes code quality Make the code cleaner or prettier. enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants