-
Notifications
You must be signed in to change notification settings - Fork 64
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
File Sharing By SAFE Data URI #210
Open
happybeing
wants to merge
9
commits into
maidsafe:master
Choose a base branch
from
happybeing:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added missing sections (small), reworded some text and improved formatting.
...to punctuation, readability and grammar.
you can easily reduce the motivation to: having links point directly network addresses makes it much more accessible and easy to share content. The point you are currently making is a little vage and could easily be done with DNS/NFS without "direct links".Options Generally don't talk about what currently is the case or how it doesn't work. that isn't the focus on the RFC, the focus is what the new feature provides.Options you can mention it in the motivation but, no need to reiterate that in the requirements.Options this shouldn't be an argument for the future but rather a description of it.Options the rest looks very good!Options one side note: I learnt today that in order to retrieve a proper network address we need to have a full DataIdentifier - So rather than saying url by hash, let's call it "url via network address" and define the "hash" as a base64 encoded serialisation of a DataIdentifier as defined in https://github.com/maidsafe/routing/blob/7c00dd14a2c4e4b2a3f7813a13edad119c0efa83/src/data/mod.rs#L111 maidsafe/routing - GitHub routing - Routing - specialised storage DHT Options by doing it this way, this will also be automatically be possible with any future additions of new data types (as they all always must be able to be addressed as DataIdentifiers within the network). So if you could update and use that instead, we are future proof in it.Options I might sound harsh or direct, so I want to state this very clearly: this is a great first draft! Thank you for doing this, it is very well done.Options I'd only like to cut down its length (as it is always easier to get people to read shorter docs rather than longer ones ;) ) and focus it a little clearer. But it is a great first go at it!Options Does my feedback make sense? Let me know if you'd like me to clarify anything or get more details or don't understand it or whatever helps you move this forward.Options One content question: so you don't want any mutable links at all?
Fix link broken by last commit! Minor formatting, typos and minor re-wording for clarity.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here's a feature request to add the ability to access files using a web URI (without going via a SAFE NFS public ID and service).
Thanks to @gnunicorn who helped a lot with this - this is actually a joint effort.