- 
                Notifications
    
You must be signed in to change notification settings  - Fork 20
 
Review of PR #362 #402
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
Review of PR #362 #402
Conversation
add createWithDurationsLl example add note about solana wallet path
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.
left some comments, and i implemented them in this commit 144a719
let me know if you agree with them, we can proceed with MI as well
| ``` | ||
| 
               | 
          ||
| Make sure to call the `withdrawFromStream` function to execute the withdrawal: | ||
| To withdraw from the stream, you can now call the `withdrawFromStream` function. | 
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.
nope, if the function is not called explicitly, the withdrawal won't be made on-chain
"Make sure to call the withdrawFromStream function to execute the withdrawal"
is more precise
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.
edited this to:
Make sure to call the `withdrawFromStream` function to execute the withdrawal on-chain.
for extra explicitness
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.
To withdraw from the stream, you can now call the
withdrawFromStreamfunction
Doesn't that mean that withdrawFromStream is called? I didn't understand why you disagree.
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.
so i see it like this:
withdrawFromStreamcalls the IDL instruction with the configured parameters- invoking 
withdrawFromStreamtriggers the withdrawal when the CLI runs - run the script with bun using the desired rpc and wallet
 
these steps together enable the withdrawal from the stream - if one is missed no withdrawal happens
when i read your version, it made me think thats the last step xD
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.
OK I understood now. See #402 (comment)
| 
           After you have removed the duplicate section, feel free to merge the PR. Your commit lgtm (other than unresolved comments).  | 
    
| 
           @smol-ninja in case you also find the note about the solana wallet config repetitive, i do too, but IMO it's fine in this kind of context  | 
    
| 
           Yeah ok its a guide anyways. What do you think about spllitting it into three pages: 
 A user may be interested into withdraw but not create so separating them into separate pages could be better.  | 
    
          
 the only problem i see with this is that they would be short pages we could also mirror the EVM version with: stream creator and stream management for   | 
    
          
 I dont think thats a problem but may be because   | 
    
          
 you are right, thats even more important  | 
    
No description provided.