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

docs(deposit): added example demonstrating the use of allocation with deposit #1071

Merged
merged 7 commits into from
Sep 6, 2024

Conversation

mgordel
Copy link
Contributor

@mgordel mgordel commented Sep 4, 2024

No description provided.

@mgordel mgordel marked this pull request as ready for review September 5, 2024 09:52
@mgordel mgordel requested review from grisha87 and jalas167 September 5, 2024 09:52
Copy link
Contributor

@grisha87 grisha87 left a comment

Choose a reason for hiding this comment

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

Everything looks good. I just came to the conclusion that there's a lot that the users have to do in order to really use the feature - for the purpose of the example you had to create few functions wrapping the complexity of the process. These are candidates to be placed in the SDK itself which we should explore as a potential feature set for the SDK.

package.json Outdated
@@ -133,7 +133,8 @@
"typedoc-plugin-markdown": "^3.17.1",
"typedoc-plugin-merge-modules": "^5.1.0",
"typedoc-theme-hierarchy": "4.1.2",
"typescript": "^5.3.3"
"typescript": "^5.3.3",
"viem": "^2.21.1"
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 should be added only to the examples?

@mgordel
Copy link
Contributor Author

mgordel commented Sep 6, 2024

definitely worth considering. I see it as golem sdk deposit create --amount --fee --expiration ...

@mgordel mgordel merged commit 19633b6 into master Sep 6, 2024
8 checks passed
@mgordel mgordel deleted the docs/JST-1080/deposit-example branch September 6, 2024 09:29
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