-
Notifications
You must be signed in to change notification settings - Fork 8
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
v2alpha1: Add sort_order field to List requests #351
Conversation
spacemesh/v2alpha1/layer.proto
Outdated
@@ -24,6 +25,7 @@ message LayerRequest { | |||
uint32 end_layer = 2; | |||
uint64 offset = 3; // adjusts the starting point for data | |||
uint64 limit = 4; // specifies max number of items to fetch | |||
optional SortOrder sort_order = 5; // specifies the sort order (default is ASC) |
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.
Do you need the optional
keyword if the default is the 0-value enum variant anyway (ASC)? Btw, I'd expect that the default (when not set) is to not sort at all. Wdyt?
ASC = 0; | ||
DESC = 1; | ||
} |
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.
Two questions:
- what about "don't sort" variant? It could save some cycles when sorting is not needed.
- does the default need to be "sorted"?
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.
We can add third option which is "don't sort" but the current implementation sorts in ascending order, and I don't want to break existing integrations for others.
# Conflicts: # release/go/spacemesh/v2alpha1/v2alpha1.pb.go # spacemesh/v2alpha1/v2alpha1.proto
Add sort_order field (enum ASC/DESC) to tx, layer and reward List requests