-
Notifications
You must be signed in to change notification settings - Fork 188
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
docs(declaration,deployment): Enhance contract declaration and deployment documentation #1359
base: main
Are you sure you want to change the base?
docs(declaration,deployment): Enhance contract declaration and deployment documentation #1359
Conversation
Oops, your pull request failed to pass the Typo tests stage :( . see the result: Please fix the typo, commit and push! |
Your preview build is ready! ✨ Check the following link in 1-2 minutes: https://starknet-io.github.io/starknet-docs/pr-1359/ . |
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.
Reviewed 3 of 4 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @omarespejel)
components/Starknet/modules/quick-start/pages/declare-a-smart-contract.adoc
line 24 at r2 (raw file):
* Deploying a contract, i.e. creating an instance of the code you previously declared. image::declaration-process.png[]
Declare contract --> Declare contract class
I'd remove the "class hash" cube since it's confusing IMO. It's an implementation detail that you refer to classes via class_hash.
Alternatively, you can, under "Declare contract class", add a row in a smaller font of "class_hash: 0x12345", and in each deploy write "deploy instance i of the class 0x12345". Though the second option sounds a bit too much to me.
components/Starknet/modules/quick-start/pages/declare-a-smart-contract.adoc
line 35 at r2 (raw file):
Declaration is the process of submitting your contract's code to the Starknet network, making it available for future deployments. It's a one-time process for each unique contract code. Think of declaration as registering a blueprint for your contract with the network. We can draw a parallel with C++ programming:
I'd remove the c++ and java sections and say:
Contract classes are analogous to classes in Object Oriented Programming. The class is defined once, and there can be many instances of it.
anything more than that is too cumbersome in a "quick-start" page
components/Starknet/modules/quick-start/pages/declare-a-smart-contract.adoc
line 49 at r2 (raw file):
Just as you can create multiple objects from a single class in C++, you can deploy multiple instances of a declared contract in Starknet. === Why Separate Declaration and Deployment?
Same here, quick-start shouldn't be about deep diving into the network. Maybe consider editing this page https://starknet-io.github.io/starknet-docs/pr-1359/architecture-and-concepts/smart-contracts/contract-classes/, and only linking to it here
components/Starknet/modules/quick-start/pages/declare-a-smart-contract.adoc
line 184 at r2 (raw file):
This hash is the identifier of the contract class in Starknet. You can think of it as the address of the contract class. You can use a block explorer like https://testnet.starkscan.co/class/0x00e68b4b07aeecc72f768b1c086d9b0aadce131a40a1067ffb92d0b480cf325d[StarkScan] to see the contract class hash in the blockchain. Using Starkli: You can use the following command to retrieve the contract class by its hash:
I think getting the raw class in a quickstart guide is too low-level.
components/Starknet/modules/quick-start/pages/deploy-a-smart-contract.adoc
line 28 at r2 (raw file):
Deployment is the process of creating a live, functional instance of your declared smart contract on the Starknet network. It's like bringing a blueprint to life by constructing an actual building. Let's use a factory production analogy:
I don't think the factory analogy should be in the quickstart guide. Potentially link to the contract classes page, but I suggest to remove this part.
components/Starknet/modules/quick-start/pages/deploy-a-smart-contract.adoc
line 58 at r2 (raw file):
---- Think of <CLASS_HASH> as the product code in our factory analogy, and <CONSTRUCTOR_INPUTS> as the specific settings for this product instance.
Same as above, in favor of removing
Your preview build is ready! ✨ Check the following link in 1-2 minutes: https://starknet-io.github.io/starknet-docs/pr-1359/ . |
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.
Reviewable status: 0 of 5 files reviewed, 6 unresolved discussions (waiting on @ArielElp)
components/Starknet/modules/quick-start/pages/declare-a-smart-contract.adoc
line 24 at r2 (raw file):
Previously, ArielElp wrote…
Declare contract --> Declare contract class
I'd remove the "class hash" cube since it's confusing IMO. It's an implementation detail that you refer to classes via class_hash.
Alternatively, you can, under "Declare contract class", add a row in a smaller font of "class_hash: 0x12345", and in each deploy write "deploy instance i of the class 0x12345". Though the second option sounds a bit too much to me.
Done. I agree to keep it simple
components/Starknet/modules/quick-start/pages/declare-a-smart-contract.adoc
line 35 at r2 (raw file):
Previously, ArielElp wrote…
I'd remove the c++ and java sections and say:
Contract classes are analogous to classes in Object Oriented Programming. The class is defined once, and there can be many instances of it.
anything more than that is too cumbersome in a "quick-start" page
Done.
components/Starknet/modules/quick-start/pages/declare-a-smart-contract.adoc
line 49 at r2 (raw file):
Previously, ArielElp wrote…
Same here, quick-start shouldn't be about deep diving into the network. Maybe consider editing this page https://starknet-io.github.io/starknet-docs/pr-1359/architecture-and-concepts/smart-contracts/contract-classes/, and only linking to it here
Done.
components/Starknet/modules/quick-start/pages/declare-a-smart-contract.adoc
line 184 at r2 (raw file):
Previously, ArielElp wrote…
I think getting the raw class in a quickstart guide is too low-level.
Done.
components/Starknet/modules/quick-start/pages/deploy-a-smart-contract.adoc
line 28 at r2 (raw file):
Previously, ArielElp wrote…
I don't think the factory analogy should be in the quickstart guide. Potentially link to the contract classes page, but I suggest to remove this part.
Done.
components/Starknet/modules/quick-start/pages/deploy-a-smart-contract.adoc
line 58 at r2 (raw file):
Previously, ArielElp wrote…
Same as above, in favor of removing
Done.
Thanks for the review @ArielElp! |
Your preview build is ready! ✨ Check the following link in 1-2 minutes: https://starknet-io.github.io/starknet-docs/pr-1359/ . |
Your preview build is ready! ✨ Check the following link in 1-2 minutes: https://starknet-io.github.io/starknet-docs/pr-1359/ . |
Hey @omarespejel, let's talk about this PR privately (closing it in the meantime). |
Partially addressed in #1416. |
Your preview build is ready! ✨ Check the following link in 1-2 minutes: https://starknet-io.github.io/starknet-docs/pr-1359/ . |
Description of the Changes
Changes to Declaration Documentation
Enhanced Introduction
Visual Representation
Context for Separation
C++ Analogy
Verification Methods
starkli class-by-hash
commandChanges to Deployment Documentation
Expanded Introduction
Visual Representation
Verification Methods
starkli class-hash-at
commandTroubleshooting Guide
PR Preview URL
https://starknet-io.github.io/starknet-docs/pr-1359/
Check List
<docs/feat/fix/chore>(optional scope): <description>
, e.g:fix: minor typos in code
This change is