Skip to content

Move caching logic to cache#2123

Merged
thomas-zahner merged 2 commits into
lycheeverse:masterfrom
thomas-zahner:move-cache-logic
Apr 7, 2026
Merged

Move caching logic to cache#2123
thomas-zahner merged 2 commits into
lycheeverse:masterfrom
thomas-zahner:move-cache-logic

Conversation

@thomas-zahner

Copy link
Copy Markdown
Member

Make Cache a struct instead of a type alias and move logic associated to caching into appropriate functions to improve readability and clarify responsibilities.

This is a follow up to #2109

@katrinafyi

katrinafyi commented Apr 2, 2026

Copy link
Copy Markdown
Member

I don't know if I like this change. It goes in the opposite direction of flatten the processing pipeline because it embeds checking as a Cache method, introducing a coupling there. I don't know if a cache should also have responsibility for executing the task. It also doesn't improve the double remap problem from the original PR (but maybe that wasn't a goal here).

edit: nvm check is a provided function, it's happy days.

@katrinafyi katrinafyi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i think it's good to centralise this logic, especially the logic around which requests/responses are bypassing the cache.

Comment thread lychee-bin/src/cache.rs
Comment thread lychee-bin/src/commands/check.rs

@katrinafyi katrinafyi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would be fine with merging this. (and it would deconflict some other PRs 🙂‍↕️)

Comment thread lychee-bin/src/cache.rs
Comment thread lychee-bin/src/cache.rs
Comment thread lychee-bin/src/cache.rs Outdated
Make `Cache` a struct instead of a type alias and move logic associated
to caching into appropriate functions to improve readability and clarify
responsibilities.
@thomas-zahner thomas-zahner merged commit 61d57d1 into lycheeverse:master Apr 7, 2026
7 checks passed
@mre mre mentioned this pull request Apr 7, 2026
Comment thread lychee-bin/src/cache.rs
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