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.

Sign up for GitHub

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

Jump to bottom

Implement automatic cache updates #115

Merged
merged 4 commits into from
Jun 26, 2020
Merged

Implement automatic cache updates #115

merged 4 commits into from
Jun 26, 2020

Conversation

ilai-deutel
Copy link
Contributor

@ilai-deutel ilai-deutel commented Apr 30, 2020 •
edited
Loading

This PR implements automatic updates of the cache.

This behavior is disabled by default and can be enabled in config.toml:

[updates]
auto_update = true
auto_update_interval_hours = 720

Fixes #90
Fixes #78

ilai-deutel
ilai-deutel commented Apr 30, 2020
View reviewed changes
tests/lib.rs Outdated Show resolved Hide resolved
@dbrgn
Copy link
Collaborator

dbrgn commented May 1, 2020 •
edited
Loading

Cool, thanks for the PR! I hope to review it sometime next week, I'm quite busy at the moment. If someone sees this and wants to speed up the process, reviews by other people certainly help 🙂

@ilai-deutel windows CI currently fails, seems to be a permission issue. Maybe you could take a look at that...

@ilai-deutel
Copy link
Contributor Author

@ilai-deutel windows CI currently fails, seems to be a permission issue. Maybe you could take a look at that...

It looks like both the utime and filetime libraries have issues with retrieving/setting modified times of Windows folders. I created a PR to fix this, alexcrichton/filetime#54

@ilai-deutel ilai-deutel mentioned this pull request May 6, 2020
Merged
@ilai-deutel
Copy link
Contributor Author

@dbrgn Fixed!

dbrgn
dbrgn requested changes May 8, 2020
View reviewed changes
Copy link
Collaborator

@dbrgn dbrgn left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for fixing this upstream, and thanks as well for the cleanup in the code. Looks better than before!

The README needs an update as well regarding the new config options. (README is getting big, we should probably use a proper documentation system in the future... Maybe mdbook.)

src/config.rs Show resolved Hide resolved
tests/lib.rs Show resolved Hide resolved
@dbrgn dbrgn mentioned this pull request May 8, 2020
Closed
dbrgn
dbrgn reviewed May 8, 2020
View reviewed changes
src/config.rs Show resolved Hide resolved
src/config.rs Show resolved Hide resolved
dbrgn
dbrgn requested changes May 19, 2020
View reviewed changes
Copy link
Collaborator

@dbrgn dbrgn left a comment

Choose a reason for hiding this comment

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

Thanks for your work so far. One last change request: Could you add the new config fields to the README as well? Then it should be ready to merge.

@ilai-deutel
Copy link
Contributor Author

Sorry it took so long, I updated the README.

@dbrgn
Copy link
Collaborator

dbrgn commented Jun 26, 2020

Thank you @ilai-deutel!

@dbrgn dbrgn merged commit d446e50 into tealdeer-rs:master Jun 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbrgn dbrgn dbrgn requested changes

Assignees
No one assigned
Labels
enhancement
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Automatic cache updates Avoid loading config twice
2 participants
@ilai-deutel @dbrgn