Various copy edits - Githubissues

OpenCyphal / specification

The Cyphal specification documents are maintained here.

https://opencyphal.org/specification

Creative Commons Attribution 4.0 International

42 stars 12 forks source link

Various copy edits #55

Closed thirtytwobits closed 5 years ago

pavel-kirienko commented 5 years ago

Any chance to move this forward?

thirtytwobits commented 5 years ago

Any chance to move this forward?

sorry. Working on this. I'll forward your questions to our tech writer, Eric.

thirtytwobits commented 5 years ago

Okay, so we seem to have consensus here. Who's actually changing what?

pavel-kirienko commented 5 years ago

Who's actually changing what?

Would you like to volunteer?

pavel-kirienko commented 5 years ago

@thirtytwobits what I'm saying is that I could go ahead and update it myself, but it would be best if it was somebody who can write decent English (i.e., not me) to avoid new issues.

veronistar commented 5 years ago

Is line 220 the last remaining open issue here? I recommend the following:

"The source of the pseudorandom data used for the pseudo-ID must aim* to produce different values for different payloads."

recommendation for a new footnote: "The number of distinct pseudo-ID values is less than the number of distinct payload values, but to avoid collision errors, this should be followed as much as possible."

pavel-kirienko commented 5 years ago

I recommend the following

This is sensible. I see no further issues with that excerpt.

recommendation for a new footnote

It seems self-contradictory: "The number of distinct pseudo-ID values is less than the number of distinct payload values" means that collisions are unavoidable, but then "to avoid collision errors <...>" implies otherwise. We could bikeshed :bike: some more, or we could just drop the footnote and hope that the reader does not need to have this spelled out for them. The main text we were discussing earlier is paramount to get right because it actually defines behaviors; elaborations and clarifications are not critical and are cheap to drop. I think.

veronistar commented 5 years ago

Let's go with removing the footnote - I'm all for removing unnecessary content.

pavel-kirienko commented 5 years ago

@veronistar are you going to be updating this, or should we wait for @thirtytwobits?

veronistar commented 5 years ago

@thirtytwobits confirmed he's good with merging this. I've just added a few commits for the outstanding comments. @pavel-kirienko - will you be doing the actual merging for this and PR #56?

pavel-kirienko commented 5 years ago

@veronistar I see that the last three commits are made in a separate branch. Will you be cherry-picking them here?

will you be doing the actual merging for this and PR #56?

Yes.

thirtytwobits commented 5 years ago

Just waiting for Pavel to sign-off then I'll merge this. Closing #56 since I ended up pulling that change into this one.

pavel-kirienko commented 5 years ago

@thirtytwobits could you please accept the seven pending suggestions I just un-resolved? I can't push to the origin.