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

Migrate Quickstart Guide to use Examples from the Javascript SDK and add more context to examples #75

Closed
wants to merge 7 commits into from

Conversation

AlwaysBCoding
Copy link
Contributor

This PR migrates the initial Quickstart guide to use examples from the Javascript SDK as well as adds more context to each of the examples to make getting started with Turnkey more approachable.

It moves the current CLI quickstart guide to a "Using the CLI" section of the docs

Copy link

vercel bot commented Jan 10, 2024

@AlwaysBCoding is attempting to deploy a commit to the Turnkey Team on Vercel.

To accomplish this, @AlwaysBCoding needs to request access to the Team.

Afterwards, an owner of the Team is required to accept their membership request.

If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account.

@AlwaysBCoding AlwaysBCoding marked this pull request as ready for review January 19, 2024 19:50
Copy link

vercel bot commented Jan 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 24, 2024 4:27pm

docs/getting-started/Quickstart-Javascript.md Outdated Show resolved Hide resolved
docs/getting-started/Quickstart-Javascript.md Outdated Show resolved Hide resolved
docs/getting-started/Quickstart-Javascript.md Outdated Show resolved Hide resolved
docs/getting-started/Quickstart-Javascript.md Outdated Show resolved Hide resolved
docs/getting-started/Quickstart-Javascript.md Outdated Show resolved Hide resolved
Comment on lines 101 to 113
```javascript
const response = await turnkeyClient.createWallet({
organizationId: TURNKEY_ORGANIZATION_ID,
type: 'ACTIVITY_TYPE_CREATE_WALLET',
timestampMs: String(Date.now()),
parameters: {
walletName: "Test Wallet 1",
accounts: []
}
})

const walletId = response.activity.result.createWalletResult.walletId;
```
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should create the account in here to simplify onboarding? That way there's a single activity rather than 2

Copy link
Contributor Author

@AlwaysBCoding AlwaysBCoding Jan 24, 2024

Choose a reason for hiding this comment

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

So my thought on this is that there's already confusion with the distinction between wallets and accounts and you can clear up some of that confusion to someone reading this guide by explicitly creating the resources in separate steps. I think in general it's a good pattern for quickstart guides to avoid creating multiple resources in a single step if there's a way to create them individually. I'd prefer leaving this as is, with the note underneath:

Note: If desired, you can also create accounts in the same API call where you create the wallet by passing in the account derivation paths as parameters to the createWallet call.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the context of a "quick" start there are competing priorities: time to value is very important because typically people want to run through this as quick as possible. The other priority is clarity.

In this case it's probably fine to leave things the way you have it if you think it's clearer. But it's "one more thing" to copy/paste; that's the trade-off.

Comment on lines 178 to 179
Make sure to replace the `unsignedTransaction` below with your own. You can use our [simple transaction generator](https://build.tx.xyz) if you need a quick transaction for testing.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs to be replaced with code to create an unsigned transaction

docs/getting-started/Quickstart-Javascript.md Outdated Show resolved Hide resolved
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.

2 participants