Skip to content
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

Documenting consensus messages #48

Closed
slowli opened this issue Apr 21, 2017 · 4 comments
Closed

Documenting consensus messages #48

slowli opened this issue Apr 21, 2017 · 4 comments
Assignees
Milestone

Comments

@slowli
Copy link
Contributor

slowli commented Apr 21, 2017

It was determined in #46 that consensus messages (e.g., Propose) are not sufficiently documented for now. Each such message could be commented like

// Request connected peers from the node `to`.
//
// ### Processing
//   * The message is authenticated by the pubkey `from`. 
//     It must be in the receiver's full node list
//   * If the message is properly authorized, the node responds with...
//
// ### Generation
// A node generates `RequestPeers` under such and such conditions...
message! {
     RequestPeers {

Note that consensus messages are slightly different from transaction messages defined by services; neither Processing, nor Generation sections can be straightforwardly translated for transaction messages (although these messages should probably be documented too). This is because tx message processing is encapsulated in the execute() method of the transaction (i.e., can be documented there); and there are no specific rules as to when ordinary tx messages are generated.

Proposed solution: I think some documentation for consensus messages is needed both here and in general Exonum docs. Message descriptions here could be useful in order to verify that messages are processed and generated as intended without needing to consult an external source. And they can be copy-pasted to the general docs if necessary.

@stanislav-tkach
Copy link
Contributor

I would like to take this issue, although I will probably need some help in order to document things properly. Still, it would be a nice opportunity to understand consensus better.

@dj8yfo
Copy link
Contributor

dj8yfo commented May 9, 2017

i assume, getting the doc actually generated depends on #17.

@defuz defuz closed this as completed in #92 May 15, 2017
@dj8yfo dj8yfo reopened this May 18, 2017
@dj8yfo
Copy link
Contributor

dj8yfo commented May 18, 2017

@defuz the comments for macro-generated messages won't actually get to rustdoc, unless #71 handles it.
So, there should be /// -> #[doc="..."] pr.
merging fast with 1 approve. #92

@stanislav-tkach
Copy link
Contributor

stanislav-tkach commented May 19, 2017

Technically #92 has been approved by @slowli, but that approval was dismissed by github after minor changes.

Additionally, the pull request was here for 7 days.

@defuz defuz closed this as completed May 24, 2017
stanislav-tkach pushed a commit to stanislav-tkach/exonum that referenced this issue Feb 10, 2018
Apply new format and update travis script
stanislav-tkach added a commit to stanislav-tkach/exonum that referenced this issue Feb 17, 2018
stanislav-tkach pushed a commit to stanislav-tkach/exonum that referenced this issue Apr 4, 2018
update link to exonum-client dependency
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants