-
Notifications
You must be signed in to change notification settings - Fork 163
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
Update and fix documentation #458
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.
Thank you!
* | ||
* const devnetRpc = createSolanaRpc(devnet('https://api.devnet.solana.com')); | ||
* const wallet = await generateKeyPairSigner(); | ||
* const keyPairBytes = new Uint8Array(JSON.parse(fs.readFileSync('path/to/solana-keypair.json', 'utf8'))); |
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.
On the one hand I like this because it is better practice than generatekeypair. On the other hand now each example requires an extra setup step and you can't just copy paste to try it out.
I'm in favor of leaving this as generatekeypair but wouldn't be opposed to adding a comment: "see wallat management on how to load a wallet" or something
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.
On the other hand don't really care as much as long as it is clear to the user
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.
Links to documentattion inside an example code-block does not look nice.
I will change to:
const wallet = await generateKeyPairSigner(); // CAUTION: this wallet is not persistent.
Since the functions only generate instructions, and there's no mentioning of "SOL airdrop on Devnet", this should be fine.
Title
Update docs
Details