Skip to content

Cache download data#75

Merged
fnattino merged 24 commits into
mainfrom
62-cache-data-fn
Feb 20, 2025
Merged

Cache download data#75
fnattino merged 24 commits into
mainfrom
62-cache-data-fn

Conversation

@fnattino
Copy link
Copy Markdown
Contributor

Closes #62

Comment thread R/utils.R
Comment thread R/valley.R
Comment thread R/osmdata.R
@fnattino fnattino marked this pull request as ready for review February 18, 2025 14:39
@fnattino fnattino requested a review from cforgaci February 18, 2025 14:45
Copy link
Copy Markdown
Contributor

@cforgaci cforgaci left a comment

Choose a reason for hiding this comment

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

Super! I pointed out two issues, one related to the use of warning() when writing to cache and the other related to a few failed osmdata tests.

Comment thread R/cache.R Outdated
Comment thread R/cache.R Outdated
Comment thread R/cache.R Outdated
Comment thread tests/testthat/test-cache.R Outdated
Comment thread R/osmdata.R
fnattino and others added 10 commits February 18, 2025 22:55
Co-authored-by: Claudiu Forgaci <33600128+cforgaci@users.noreply.github.com>
Co-authored-by: Claudiu Forgaci <33600128+cforgaci@users.noreply.github.com>
Co-authored-by: Claudiu Forgaci <33600128+cforgaci@users.noreply.github.com>
Co-authored-by: Claudiu Forgaci <33600128+cforgaci@users.noreply.github.com>
@fnattino
Copy link
Copy Markdown
Contributor Author

Hi @cforgaci, thanks for the useful comments, I have hopefully addressed those. Can you have a final look and potentially confirm whether all tests succeed locally on your system as well?

@fnattino fnattino requested a review from cforgaci February 19, 2025 12:12
Copy link
Copy Markdown
Contributor

@cforgaci cforgaci left a comment

Choose a reason for hiding this comment

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

Looks good. All tests pass locally.

@fnattino fnattino merged commit 47b6395 into main Feb 20, 2025
@fnattino fnattino deleted the 62-cache-data-fn branch February 20, 2025 08:59
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.

Caching download data

2 participants