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

[suggestion] Make a query builder and remove the boilerplate request_with_* methods #3124

Closed
pesterev opened this issue Feb 8, 2023 · 4 comments
Assignees
Labels
Enhancement New feature or request good first issue Good for newcomers iroha2-dev The re-implementation of a BFT hyperledger in RUST QA-confirmed This bug is reproduced and needs a fix

Comments

@pesterev
Copy link
Contributor

pesterev commented Feb 8, 2023

Feature request

My suggestion is to make a builder and to get rid of these Client's request_with_* methods.

Motivation

We have many boilerplate Client's query methods.

Who can help?

@appetrosyan @pesterev

@pesterev pesterev added Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST labels Feb 8, 2023
@pesterev pesterev self-assigned this Feb 8, 2023
@pesterev pesterev added the good first issue Good for newcomers label Feb 8, 2023
@appetrosyan
Copy link
Contributor

Good idea. Let's make sure we cover all of the patterns in the code.

@pesterev
Copy link
Contributor Author

pesterev commented Feb 8, 2023

Another suggestion is to use "default parameters".

@appetrosyan
Copy link
Contributor

appetrosyan commented Feb 8, 2023

Personally, this is up for debate. The builder pattern is common-enough in our code base not to surprise new users. While in a vacuum I'd prefer having a struct and use the ..Default::default() in a struct literal, because we already make good use of the builder pattern elsewhere it'd lead to less surprise.

@Erigara unless you have a preference for Default ellipses, I'd consider using the builder pattern as described in the issue.

@Erigara
Copy link
Contributor

Erigara commented Feb 8, 2023

@appetrosyan @pesterev i don't insist on ..Default::default(), i agree that we already have enough places where we use builders, so we can keep doing that.
I just want to provide options for us to consider.

pesterev added a commit to pesterev/iroha that referenced this issue Oct 27, 2023
…ient API

Signed-off-by: Vladimir Pesterev <8786922+pesterev@users.noreply.github.com>
pesterev added a commit to pesterev/iroha that referenced this issue Oct 27, 2023
…ient API

Signed-off-by: Vladimir Pesterev <8786922+pesterev@users.noreply.github.com>
pesterev added a commit to pesterev/iroha that referenced this issue Oct 27, 2023
…ient API

Signed-off-by: Vladimir Pesterev <8786922+pesterev@users.noreply.github.com>
pesterev added a commit to pesterev/iroha that referenced this issue Oct 31, 2023
…ient API

Signed-off-by: Vladimir Pesterev <8786922+pesterev@users.noreply.github.com>
pesterev added a commit to pesterev/iroha that referenced this issue Oct 31, 2023
…ient API

Signed-off-by: Vladimir Pesterev <8786922+pesterev@users.noreply.github.com>
pesterev added a commit to pesterev/iroha that referenced this issue Oct 31, 2023
…ient API

Signed-off-by: Vladimir Pesterev <8786922+pesterev@users.noreply.github.com>
pesterev added a commit to pesterev/iroha that referenced this issue Oct 31, 2023
…ient API

Signed-off-by: Vladimir Pesterev <8786922+pesterev@users.noreply.github.com>
pesterev added a commit that referenced this issue Nov 1, 2023
Signed-off-by: Vladimir Pesterev <8786922+pesterev@users.noreply.github.com>
@timofeevmd timofeevmd self-assigned this Dec 8, 2023
@timofeevmd timofeevmd added the QA-confirmed This bug is reproduced and needs a fix label Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request good first issue Good for newcomers iroha2-dev The re-implementation of a BFT hyperledger in RUST QA-confirmed This bug is reproduced and needs a fix
Projects
None yet
Development

No branches or pull requests

4 participants