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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: better cache handling without external modules #11

Conversation

naranjamecanica
Copy link
Contributor

@naranjamecanica naranjamecanica commented Nov 21, 2023

I've took your PR and made a version without any external modules. For readability i've made a seperate cache.ts file which handles all the caching logic. Hope you can merge it! 馃憤馃徎

@naranjamecanica naranjamecanica changed the title Feat: better cache handling Feat: better cache handling without external modules Nov 21, 2023
@vHeemstra
Copy link
Owner

Good idea to separate it into its own file.

I'll have a better look soon.

Side note: When cache is used for a input file, we can't show/log the resulting output files and (size) improvements. I was thinking about a way to store this info/stats with the cache entry, so it can be read and used in the result log. Is that useful you think?

@naranjamecanica
Copy link
Contributor Author

That's a nice idea about the stats, I can look into that to make a start.

One other thing I thought of that's not handled well right now is that if you change the compression settings the cache should also be discarded. I'll look into that later tonight of tomorrow!

@vHeemstra
Copy link
Owner

Great!

Yeah that's what I liked about the cache package you introduced. It uses a hash of a JSON.stringifyd object of all the options as the file name to store the cache entries. This way, if the options change, it will look for a different file for the cache entries.

We might use a similar hash for the name of a subfolder? In this folder there will be the entries json as well as the copies of the output files. In this way, you could easily switch between configs and still have caching on all of those.

@vHeemstra vHeemstra changed the base branch from main to improve-cache-handling November 21, 2023 18:31
@vHeemstra vHeemstra merged commit 05bd001 into vHeemstra:improve-cache-handling Nov 21, 2023
2 checks passed
@vHeemstra
Copy link
Owner

I merged your changes into the improve-cache-handling branch, so we can work on it on that branch until it's ready to merge with main.

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.

None yet

2 participants