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

Change API OfferInfo volume/minVolume proto fields to string [#4] #6060

Merged

Conversation

ghubstan
Copy link
Contributor

@ghubstan ghubstan commented Feb 18, 2022

As longs, they created confusion and extra client side code for handling display of altcoin volumes to 2 decimal places, and fiat volumes to 0 decimal places.

  • Change OfferInfo proto's volume fields' type to string.
  • Adjust cli module to grpc.proto's volume & minVolume field type change (string).
  • Add test asserts on volume/min-volume strings from server.

Based on branch 3-change-grpc-proto-triggerprice-to-string, PR #6057

This commit adds comments to grpc.proto to be pulled into a new API
reference doc.  Some of the comments highlight issues to be resolved
in following PRs.

The main focus is on documenting gRPC Offers service definitions in
grpc.proto:

- Comment each field in the Offers service's messages.
- Add TODO comments about fixing field defs with usability &
  consistency problems.

Other API reference doc related changes to grpc.proto:

- Comment each gRPC service's rpc method definitions, however briefly.
- Improve some existing rpc method comments.
We will consider each problematic number field one at a time.

Note: changing field types in following commits will break
the API's backward compatibility, but devs agree it's OK since
it is still in beta.
The new console output formatting (tbl) api is working, and the
deprecated classes just add maintenance work.

Update affected test cases.
Much less ambiguous field value at cost of breaking backward compat in API.
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK - Changes are looking fine 👍

@ripcurlx ripcurlx added this to the v1.8.4 milestone Feb 28, 2022
@ripcurlx ripcurlx merged commit 532efcc into bisq-network:master Feb 28, 2022
@ghubstan ghubstan deleted the 4-change-grpc-proto-volume-to-string branch February 28, 2022 13:31
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