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

Documentation about API for keys and pool methods. #133

Merged
merged 13 commits into from
Feb 24, 2022
Merged

Conversation

lampkin-diet
Copy link
Contributor

No description provided.

@askolesov
Copy link
Contributor

@ankurdotb Looks like there will be several ADRs that are cheqd related but about VDR Tools. What is the right place for them?

@ankurdotb
Copy link
Contributor

@ankurdotb Looks like there will be several ADRs that are cheqd related but about VDR Tools. What is the right place for them?

I don't see this document as an ADR, it's more a developer documentation for how to use the software. Good question on whether it should be here though: if it's about how to use VDR Tools, then I'd expect it to be im VDR Docs but we can link to those docs from ours. Is there doc site similar to docs.cheqd.io for VDR Tools?

Copy link
Contributor

@ankurdotb ankurdotb left a comment

Choose a reason for hiding this comment

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

We should discuss this...it seems there are things being implemented here that are duplicates of what cheqd Cosmos CLI is meant to achieve. To me, this would go against what we decided in the ADR for CLI tools: https://docs.cheqd.io/node/architecture/adr-list/adr-003-cli-tools

More broadly though, VDR Tools SDK should assume the network is already up and running, and no configuration tasks are expected to be carried out from VDR Tools. Broadcast as a concept also doesn't make sense to me.

docs/pool_connection.md Outdated Show resolved Hide resolved
docs/pool_connection.md Outdated Show resolved Hide resolved
docs/pool_connection.md Outdated Show resolved Hide resolved
docs/pool_connection.md Outdated Show resolved Hide resolved
docs/pool_connection.md Outdated Show resolved Hide resolved
docs/pool_connection.md Outdated Show resolved Hide resolved
Copy link
Contributor

@ankurdotb ankurdotb left a comment

Choose a reason for hiding this comment

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

Minor comments on method names, would be good to understand if we're sticking with this for now and refactoring later (this is fine by me if that's the decision on the VDR Tools side / it's not easy to change right now).

Otherwise, I've resolved a whole bunch of other comments after the clarification regarding usage of some methods.

Last thing remaining besides confirming method names is there's a minor update needed for account names, where the prefix has now changed.

Looks good otherwise! Thanks @anikitinDSR

docs/pool_connection.md Outdated Show resolved Hide resolved
@ankurdotb ankurdotb marked this pull request as draft November 12, 2021 19:10
Andrew Nikitin added 4 commits February 24, 2022 00:06
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
@ankurdotb ankurdotb marked this pull request as ready for review February 24, 2022 00:12
@ankurdotb ankurdotb merged commit 6820e4c into main Feb 24, 2022
@ankurdotb ankurdotb deleted the pool_connection_md branch February 24, 2022 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants