Skip to content

Conversation

@nicocartesi
Copy link
Contributor

@nicocartesi nicocartesi commented Jul 25, 2023

This new section describes a way to customise a DApp by editing the existing examples, particularly the Calculator app. It has been pointed out that create-dapp.sh is not working optimally, so before the Sunodo release, we advise building DApps in this way.

Copy link

@xdaniortega xdaniortega left a comment

Choose a reason for hiding this comment

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

Great improvement to the documentation! Here are my thoughts:

  • We have to prepare a good template dApp to allow users to start easily and don't make them understand the logic of an example dApps. Examples should be used to further extend the capabilities of what a developer could do. But it's great to have this tutorial before having the perfect template.
  • The first goal is to build a very simple and working example, it adds complexity to clone and use one example and then importing another library to use Fortune.
  • I don't know what fortune is.
  • I don't know what subprocess is for.
  • If I am planning to build my application from an existing example, I would go for the less modifications possible. Maybe cloning, changing example name and executing everything (back and front) to see that everything works. Then modify backend logic.
  • I loved the section where you show to check dApp address

@nicocartesi
Copy link
Contributor Author

Great improvement to the documentation! Here are my thoughts:

  • We have to prepare a good template dApp to allow users to start easily and don't make them understand the logic of an example dApps. Examples should be used to further extend the capabilities of what a developer could do. But it's great to have this tutorial before having the perfect template.
  • The first goal is to build a very simple and working example, it adds complexity to clone and use one example and then importing another library to use Fortune.
  • I don't know what fortune is.
  • I don't know what subprocess is for.
  • If I am planning to build my application from an existing example, I would go for the less modifications possible. Maybe cloning, changing example name and executing everything (back and front) to see that everything works. Then modify backend logic.
  • I loved the section where you show to check dApp address

Awesome feedback, thanks @DaniOrtegaB !

@nicocartesi nicocartesi changed the base branch from main to next July 28, 2023 13:20
@felipefg
Copy link

felipefg commented Aug 4, 2023

I'd say this step-by-step is great, but kind of unfortunate that we have to work around the create-dapp.sh. Lyno has made a pull request to the rollups-examples repo (cartesi/rollups-examples#28) to make create-dapp.sh work with the docker-riscv build system. But it hasn't been merged yet.

I have a suggestion on the style of explaining the changes. Instead of saying in function <func> delete <code> and add <code>, I'd suggest being something like "Change the <func> function so it reads like this: <code>", and then only displaying the full final code for the function, including the declaration line. The risk I'd try to avoid is for the reader to see the old piece of code in the documentation without reading the surrounding text and not realize it is not part of the explanation, but has to be removed. But that is a minor suggestion, really.

@nicocartesi
Copy link
Contributor Author

Thanks @felipefg ! I amended the steps as you suggested. Agree, it's a workaround so once the script has been fixed, I will add it in front of this tutorial. So we can show two ways of customising: the preferred one using the script and this workaround.

@felipefg
Copy link

felipefg commented Aug 4, 2023

Thanks! Looks great for merging.

@tuler tuler merged commit 0c5e8a5 into cartesi:next Aug 4, 2023
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.

4 participants