-
Notifications
You must be signed in to change notification settings - Fork 0
feat(hashets): add WrapPrecomputedFS #3
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
Conversation
|
the example project in this MR depends on #2 as it calls i can also create a dedicated MR for the example project in order to keep the focus of this MR on the implementation of |
mavolin
left a comment
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.
I really appreciate the time you took writing the example. Therefore I feel bad for saying this, but I feel the example is just too big (9 files) to be sensible. I realise there are people who prefer thorough examples, but I think if you were to add this method of hashing to the readme and quickly explain the tradeoffs/benefits compared to the other methods (mimicking the style of the other examples), it would prove just as valueable.
I think this has two advantages:
- Since there are no examples for the other methods of hashing, people might overlook this example.
- The examples in the readme are much shorter and can hence be understood in far less time. I've had the experience that mini-project examples like this take far more time to understand, since there is no "guiding hand" that explains the idea; whereas the readme can sort of "narrate" what is happening, often requiring less code and therefore less effort to understand the idea behind this example.
I'm obviously fine HashFS itself.
|
i have remove the example app and instead added an additional section to the README.md explaining the usecase and drawbacks |
mavolin
left a comment
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.
Good renaming, thank you for readme section. Some minor changes for clarity, and then we have it. :)
`WrapFS` implementation where the mappings are compute elsewhere, e.g. during build time.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v1 #3 +/- ##
===========================================
+ Coverage 27.58% 57.64% +30.06%
===========================================
Files 5 3 -2
Lines 348 170 -178
===========================================
+ Hits 96 98 +2
+ Misses 231 51 -180
Partials 21 21
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@UiP9AV6Y thank you for your PRs, I've created a release :) |
filesystem implementation usable for scenarios where consumers request resources with their hashed name.