WalletWasabi / WabiSabi

MIT License
104 stars 28 forks source link

Add StatusRequest and Coordinator parameters explanations #68

Closed nopara73 closed 4 years ago

nopara73 commented 4 years ago

Sorry, but I don't understand.

nothingmuch commented 4 years ago

the lines i link to talk about how the details of the status message are not yet decided, and discussion is ongoing in the github thread, but now you've added them... i was trying to think of a suggested edit but didn't come up with anything:

i couldn't decide which makes the most sense overall, but right now it's kind of redundant and internally inconsistent

nopara73 commented 4 years ago

the lines i link to talk about

Is broken. You're linking to L63-L65, but the PR starts at L107 thus your link brings up the whole diff between this PR and the master.

talk about how the details of the status message are not yet decided

We can't decide before amount organization takes place, but...

right now it's kind of redundant and internally inconsistent

...but has no chance to understand the rest of the diagrams without knowing what's going on with the status messages, thus it's not redundant. In fact to resolve the inconsistency we should remove the "the lines i link to talk about how the details of the status message are not yet decided". And this way it'll only be incomplete, but not inconsistent and when amounts come in we can extend and modify it accordingly.

nothingmuch commented 4 years ago

yeah I'm talking about lines that should have been changed but weren't, they talk about how what you added as if it's not yet added

On Mon, 29 Jun 2020, 14:26 nopara73, notifications@github.com wrote:

the lines i link to talk about

Is broken. You're linking to L63-L65, but the PR starts at L107 thus your link brings up the whole diff between this PR and the master.

talk about how the details of the status message are not yet decided

We can't decide before amount organization takes place, but...

right now it's kind of redundant and internally inconsistent

...but has no chance to understand the rest of the diagrams without knowing what's going on with the status messages, thus it's not redundant. In fact to resolve the inconsistency we should remove the "the lines i link to talk about how the details of the status message are not yet decided". And this way it'll only be incomplete, but not inconsistent and when amounts come in we can extend and modify it accordingly.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/zkSNACKs/WabiSabi/pull/68#issuecomment-651051891, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADPIT56FOEUKEOZBLNKFTRZB27LANCNFSM4OJGQEDA .

nopara73 commented 4 years ago

the text you added could be moved up from the interaction diagram section to this section

Not sure. It'd be more difficult to read without the relevant diagram IMO.

the previous text can just be removed altogether, because it doesn't really add anything and the document flow can be improved later

What do you refer to as "previous text"?

nothingmuch commented 4 years ago

What do you refer to as "previous text"?

the text i linked to... i should have said "out of date text"