Skip to content
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

Updated the "Starting a Project" docs to use the new Simple Game Dapp #894

Merged
merged 72 commits into from
Dec 20, 2023
Merged

Updated the "Starting a Project" docs to use the new Simple Game Dapp #894

merged 72 commits into from
Dec 20, 2023

Conversation

kbennett2000
Copy link
Contributor

@kbennett2000 kbennett2000 commented Dec 11, 2023

Updated the "Starting a Project" docs to use the new Simple Game Dapp

Copy link
Contributor

mergify bot commented Dec 11, 2023

⚠️ The sha of the head commit of this PR conflicts with #875. Mergify cannot evaluate rules on this PR. ⚠️

Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

Lots of good stuff!

Let's use fenced code blocks throughout. And I have various other comments I'd like you to consider.

I covered a lot of the same material in #881 ; I suppose I should close #881 in favor of this one, but first I want to make sure the essential material from there is covered.

main/guides/getting-started/start-a-project.md Outdated Show resolved Hide resolved
main/guides/getting-started/start-a-project.md Outdated Show resolved Hide resolved
3. ```sh secondary style3
# Terminal 3: web user interface
```
# Installing Curl
Copy link
Member

Choose a reason for hiding this comment

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

Let's demote installing curl to a troubleshooting box that includes the error message for when it's missing. I'm pretty sure it's not necessary for most of our audience.

Are we sure we want to use curl at all rather than just assuming folks know how to use a browser to download stuff?

Copy link
Contributor Author

@kbennett2000 kbennett2000 Dec 12, 2023

Choose a reason for hiding this comment

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

I don't see a way to install NVM via a browser. If you know of something point me in the right direction? I wanted devs to use NVM as it makes switching node versions trivial, and it seems like we have some very specific Node version dependencies which will likely shift over time.

Copy link
Member

Choose a reason for hiding this comment

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

I don't see a way to install NVM via a browser.

The nvm install docs start with:

To install or update nvm, you should run the install script.

Folks can use their web browser to download the install script.

I wanted devs to use NVM as it makes switching node versions trivial, and it seems like we have some very specific Node version dependencies which will likely shift over time.

I'm cool with recommending nvm. I use it myself. Something like:


Install an LTS version of Node.js; for example, install nvm and use:

nvm install --lts
nvm use --lts
node --version # for example: v18.17.1

That should be enough for our target audience.

Copy link
Member

Choose a reason for hiding this comment

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

it seems like we have some very specific Node version dependencies which will likely shift over time

With occasional exceptions such as Agoric/agoric-sdk#8599 , our policy is: any LTS version of node should work.

I'm inclined to put troubleshooting stuff like 8599 in details boxes:

::: details Troubleshooting Symbol.dispose error from node v18.19.0
Node v18.19.0 has a breadking change that results in...
```
Removing intrinsics.Symbol.dispose
failed to delete intrinsics.Symbol.dispose (TypeError#1)
TypeError#1: Cannot delete property 'dispose' of function Symbol() { [native code] }
```
This is a known issue, but updating this app with the fix may take some
time. Meanwhile, please downgrade node to an earlier version.
For reference:
- [Node 18\.19 breaks release\-mainnet1B branch · agoric\-sdk \#8599](https://github.com/Agoric/agoric-sdk/issues/8599)
- [feat\(ses\): tame Symbol so whitelist works by erights · endojs \#1579](https://github.com/endojs/endo/pull/1579)
:::

main/guides/getting-started/start-a-project.md Outdated Show resolved Hide resolved
main/guides/getting-started/start-a-project.md Outdated Show resolved Hide resolved
main/guides/getting-started/start-a-project.md Outdated Show resolved Hide resolved
main/guides/getting-started/start-a-project.md Outdated Show resolved Hide resolved
@dckc
Copy link
Member

dckc commented Dec 12, 2023

Why add pointToLocalhost.png and the other 2 images in main/guides/assets/?

@dckc
Copy link
Member

dckc commented Dec 12, 2023

@kbennett2000 I suggest adding to the PR description:

fixes: #726

PR #894 -- #894

Changes Made:
- Fenced code blocks added
- Updated `yarn start` command
- Large screenshots rescaled to 800px max
- Screenshots updated for all updated commands
- Added caution phrasing around mnemonic phrase management
- Removed selection of "Cosmos Hub" from Keplr wallet
- Removed personal email address, replaced with "[email protected]" under How to Get Help section
Added line breaks between text and code blocks
@@ -1,139 +1,292 @@
# Starting a Project
# Your First Agoric Dapp
Getting your first Agoric application up and running!
Copy link
Member

Choose a reason for hiding this comment

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

Dapp and application... do those mean the same thing? Are you using different words on purpose?

Is this not a term we want to define on first usage?

For example:

Welcome! We'll start with a basic _dapp_; that is: a JavaScript contract and associated UI.

@kbennett2000 kbennett2000 requested a review from dckc December 19, 2023 21:20
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

this is going to help so much!

There's probably more to do, but I think all the critical stuff is here.

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.

transition getting started from ag-solo to smart wallet + RPC
2 participants