Skip to content

Conversation

philss
Copy link

@philss philss commented Dec 7, 2022

This makes the usage simpler and faster.

This makes the usage simplier and faster.
Copy link
Member

@jonatanklosko jonatanklosko left a comment

Choose a reason for hiding this comment

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

We could also make a HEAD request and compare E-tag, since for revision like main the file may change, that's what we do in the other project. But the reference implementation doesn't do it either, so it's probably fine, we can revisit if we run into any actual issues!

@jonatanklosko
Copy link
Member

Oh actually, having a brief look at the cached_path rust package, comparing E-tag seems to be exactly what it does?

@philss
Copy link
Author

philss commented Dec 7, 2022

@jonatanklosko humm, interesting! I think I'm going to move this responsibility to the HTTP client then 🤔
I'm going to try there.

@philss
Copy link
Author

philss commented Dec 7, 2022

I think I'm going to move this responsibility to the HTTP client then

Actually maybe not. Haha. I will be out for the next two hours, but I change this after I'm back.

@philss
Copy link
Author

philss commented Dec 7, 2022

@jonatanklosko it's not pretty but now works with E-tags :)

Copy link
Member

@jonatanklosko jonatanklosko left a comment

Choose a reason for hiding this comment

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

Minor comments, but looks good to me!

@jonatanklosko
Copy link
Member

🚢 🐈

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.

2 participants