-
Notifications
You must be signed in to change notification settings - Fork 441
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
Add README
to DNS example
#1542
Conversation
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.
Thanks for you PR, but it needs some fixes.
Also, can you wrap everything to ~90 characters?
examples/dns/README.md
Outdated
@@ -0,0 +1,33 @@ | |||
# DNS Smart contract | |||
|
|||
The DNS smart contract is our showcase for executing other smart contracts on-chain. |
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.
This contract doesn't execute any other contracts, it's self contained
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.
Sure, changed it
examples/dns/README.md
Outdated
|
||
|
||
|
||
> This is a port from the blog post's ink! 1.0 based version of the contract to ink! 2.0. |
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.
This is not relevant, and is also outdated. Our current stable release is ink! 3.0, and the contract compiles with the ink! 4.0-beta.
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.
Okay, I've mentioned it in the readme. Also, I will try compiling it against 4.0 beta
examples/dns/README.md
Outdated
### Other functionalities to be implemented: | ||
- Transfer bidding |
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 don't think anybody is going to update this example tbh
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.
Sure, I'll remove it then for now
examples/dns/README.md
Outdated
> __Note:__<br/> | ||
> Depending on your Substrate version you might encounter [a ink_lang dependency error](https://github.com/paritytech/ink/issues/506) which is yet to address. |
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.
Where did you get this? This issue has been closed for 2+ years
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 came across this when compiling the contract. While compiling it gives an error of 'ink_lang'
But as mentioned above I was compiling it using the 3.0 version.
examples/dns/README.md
Outdated
The main function of this contract is domain name resolution which refers to the retrieval of numeric values corresponding to readable and easily memorable names such as "polka.dot" which can be used to facilitate transfers, voting and DApp-related operations instead of resorting to long IP addresses that are hard to remember. | ||
<br> | ||
|
||
![Image](dns_diagram.png) |
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.
Thanks for the diagram. Can you make a examples/dns/.images
folder and put it in there instead?
examples/dns/README.md
Outdated
[this blog post on medium](https://medium.com/@chainx_org/secure-and-decentralized-polkadot-domain-name-system-e06c35c2a48d). | ||
|
||
### Description | ||
The main function of this contract is domain name resolution which refers to the retrieval of numeric values corresponding to readable and easily memorable names such as "polka.dot" which can be used to facilitate transfers, voting and DApp-related operations instead of resorting to long IP addresses that are hard to remember. |
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.
Can you update this to maybe show an example of how this translation would work?
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.
This exact paragraph was written in the comments itself of dns.sol
and I've grabbed it from the file. But I think we can make it more clear and show what are the parameters this contract uses.
Codecov Report
@@ Coverage Diff @@
## master #1542 +/- ##
==========================================
- Coverage 71.62% 65.36% -6.27%
==========================================
Files 205 205
Lines 6295 6288 -7
==========================================
- Hits 4509 4110 -399
- Misses 1786 2178 +392
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Hii @HCastano I have made some changes, please review it and let me know! Thank you |
Hey @Irene-123 so taking another looks it seems like the info in the README is already mostly covered by the doc comments in the code itself. I'm gonna close it for that reason. Thanks for the effort though! |
Hello there, I'm a blockchain enthusiast and contributing to open-source projects lately. I've been using substrate lately for my college projects and found that this great DNS smart contract doesn't have a README, so I've added it. Thank you!