Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

use PATH_CACHE consistently within determinator.rb #86

Closed
wants to merge 2 commits into from

Conversation

klandergren
Copy link

Previously, PATH_CACHE was only used by formatted_last_modified_date and
not by last_modified_at. This resulted in last modified time being
re-determined when using:

{{ page.last_modified_at }}

but not when using:

{% last_modified_at %}

This change aligns the behavior and, as a by product, improves site reset
render performance for users of page.last_modified_at. Initial site
generation render performance is unaffected. The tradeoff is that data can get
stale in the cache, and repo information is not reread on reset.

More information available in #85

I did not test extensively but did verify that bundle exec rake spec passed and bundle exec rake rubocop did not introduce any new issues.

Previously, `PATH_CACHE` was only used by `formatted_last_modified_date` and
not by `last_modified_at`. This resulted in last modified time being
re-determined when using:

{{ page.last_modified_at }}

but not when using:

{% last_modified_at %}

This change aligns the behavior and, as a by product, improves site reset
render performance for users of `page.last_modified_at`. Initial site
generation render performance is unaffected. The tradeoff is that data can get
stale in the cache, and repo information is not reread on reset.
`item.path` returns the absolute path to a file, whereas other instantiations
of `Determinator`, like in `tag.rb`, use a relative path. by using a relative
path in both places we increase the likelihood of cache hits.
@gjtorikian
Copy link
Owner

👋 Unfortunately, my GitHub notifications for this repository was oddly turned off. I take my responsibility to accepting patches seriously; however, I am just one person, with a very busy life off of GitHub.com! I'm going to close this PR because I like to keep my TODO list scoped and reasonable. This does not mean I will not accept this patch! Rather, it's a way of me asking you if you're still using this gem, and whether you want this PR merged. Just comment back and I'll prioritize it. I assume silence means I can keep it closed. Thanks!

this was an automated copy-paste

@gjtorikian gjtorikian closed this Jun 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants