search

status-im / specs

Specifications for Status clients.
https://specs.status.im/
MIT License
14 stars 14 forks source link

Move client to stable #93

Closed oskarth closed 4 years ago

oskarth commented 4 years ago

This PR moves client spec from draft to stable. This means it is an accurate description of what is currently in production for the v1 app.

If we want to do wider changes in the future these should come in the form of SIP issues and PRs, as indicated in the README.

Changes to this spec can still occur but they should be cosmetic ones, errata and clarifications.

Please review with anything factually incorrect or add any relevant data. I want to get this merged over the next 24-48h. This will ensure we have a stable set of specs that accurately reflects what is in production, and then we can proceed from there.

oskarth commented 4 years ago

@corpetty Thanks for the feedback! Could you please do the review in-line? The line numbers are quite confusing and they also change. It is much easier to have a conversation and do suggestions etc in a specific locale. E.g. I noticed you pushed a commit with fixes, but I have no idea what that fixes of your comments.

I know the file diff is a bit off, so you gotta expand to see lines not changed. Even that doesn't work all the time. I suggest pushing a commit with the comment in-line at the appropriate place, then we can comment and discuss on it in-line. WDYT?

jakubgs commented 4 years ago 
3. Mailservers (servers and clients)

What does servers and clients mean? A "mailserver" is just a server.

we RECOMMEND you implement a mailserver client mode

So isn't a mailserver client mode just a mobile node? I don't get this terminology.

`eth.beta` is the current group of peers

No, it's eth.prod.

There's still two TODO comments in the text. And why are you breaking lines yourself? Looks weird.

corpetty commented 4 years ago

And why are you breaking lines yourself?

pretty sure this is a "tabs vs spaces" type argument

oskarth commented 4 years ago

Answered mailserver/client in Discord. Added eth.prod and removed TODOs

corpetty commented 4 years ago

How do we suggest cleaning up the Node discovery and roles section as that seems to be the most unclear?

corpetty commented 4 years ago

This doc lacks most uses outside of chat, should they be included?

oskarth commented 4 years ago

This doc lacks most uses outside of chat, should they be included?

We probably should, at least mention them and then full specs can come later. E.g. we need the wallet tx stuff way more specified

corpetty commented 4 years ago

so we have:

Correct?

corpetty commented 4 years ago

@oskarth I made what I think to be the last edit for this. Please review that commit and merge if you feel it is sufficient.