-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor should_use_ephemeral_cache #7262
refactor should_use_ephemeral_cache #7262
Conversation
34148bf
to
283eb23
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
283eb23
to
e638e24
Compare
Thanks @sbidoul! To answer your question about news files - a |
Thanks @chrahunt and @xavfernandez for the swift review and merge. I continue in #7268. |
Towards #6852
Following @chrahunt advice that no PR is small enough, I start with this refactoring of
should_use_ephemeral_cache
intoshould_build
andshould_cache
. The new functions have independent tests. Next step will be to removeshould_use_ephemeral_cache
.It's not exactly tiny, but it should hopefully be easy to understand.
I think all review comments from #6853 have been handled, including a #6853 (comment) (in a separate commit).
I have one question: which kind of newsfragment file should I create for a refactor like this?