search

ExchangeUnion / xud

Exchange Union Daemon 🔁 ⚡️
https://exchangeunion.com
GNU Affero General Public License v3.0
115 stars 44 forks source link

gRPC+CLI renaming "list"/"get" cleanup #681

Closed kilrau closed 5 years ago

kilrau commented 6 years ago

This issue is about renaming list/get calls in a consistent manner. I know we all have our feelings about this and especially all the renaming. Brought that up because I believe it's still fixable now. Please leave comments.

Motivation: Let's analyze it from a new dev/CLI user perspective: it looks inconsistent how we use get and list e.g. getOrders and listTrades. Especially on CLI usage one tends to pick the wrong one.

Option 1: All list -> get. E.g. listPeers, listPairs to getPeers, getPairs. I'd go for get since it's the more generic form and will always make sense for a dev implementing the gRPC. IMO the CLI just represents data it gets from the gRPC/xud in the most suitable form. That might be a list, a table, a JSON or a circle. Good: consistent Bad: differs from lnd, renaming effort

Option 2: The lnd way: get only for info calls (getInfo), rest is consistently list. Good: follows lnd Bad: not consistent

sangaman commented 6 years ago

I've voiced my opinion in related issues but I'll say again.

"List" definitely does not make sense for fetching single objects. So we definitely should not have ListInfo for example, but also not ListNodeInfo for a single node.

Using "list" for lists and "get" for single objects is pretty standard I think and consistent with lnd as well as the bitcoind API.

I think the only one that might be worth changing is GetOrders to ListOrders - the data format of the response for that call actually changed somewhat recently and now it's basically a list of orders lists. I know @offerm had suggested changing this earlier, so if we want to change that and then stick with "list" for plural and "get" for single then I'm fine with that.

ImmanuelSegol commented 6 years ago

@sangaman @kilrau How should I move forward on this? Personally, I think we should go for get but @sangaman has a good point and I agree with him.

kilrau commented 6 years ago

What @sangaman describes is Option 2 which is fine with me: only one thing to do: rename GetOrders to ListOrders. Let's go with this one and we are in line with our role model lnd :)

And for future calls we remember the logic @sangaman described: multiple elements/plural like orders or trades: ListX, one element like info: GetX.