shared/tunnelbroker_messages/src/messages/send_confirmation.rs
1–2 ↗	(On Diff #31961)	You can use this syntax to apply doc comments to the item they are in. So in this case this would be applied to the `send_confirmation` module. This is useful for module-level documentation.
6–14 ↗	(On Diff #31961)	This should probably be moved to be above `MessageSentStatus`. You could also move the variant specific comments above the variant definition e.g. enum MessageSentStatus { /// Message with id was processed by Tunnelbroker Success(String) ... }
45 ↗	(On Diff #31961)	Shouldn't this be `SendConfirmation`?

michal added inline comments.Oct 13 2023, 7:04 AM

shared/tunnelbroker_messages/src/messages/send_confirmation.rs
33–36 ↗	(On Diff #31961)	Nit: "confirmation" sounds like the message already succeeded but this also contains message send failures (not entirely happy with my name suggestion too tho)

address review

shared/tunnelbroker_messages/src/messages/send_confirmation.rs
1–2 ↗	(On Diff #31961)	I'll do it in separate diff and apply this comment to all files with messages
45 ↗	(On Diff #31961)	oh right, I probably messed something up while rebasing, thanks

kamil retitled this revision from [Tunnelbroker] add SendConfirmation message to [Tunnelbroker] add MessageToDeviceRequestStatus message.Oct 18 2023, 1:39 AM

Harbormaster completed remote builds in B23323: Diff 32133.Oct 18 2023, 1:47 AM

michal accepted this revision.Oct 18 2023, 2:56 AM

This revision is now accepted and ready to land.Oct 18 2023, 2:56 AM

Closed by commit rCOMM50312d2074b4: [Tunnelbroker] add MessageToDeviceRequestStatus message (authored by kamil). · Explain WhyOct 18 2023, 3:10 AM

This revision was automatically updated to reflect the committed changes.

kamil added a commit: rCOMM50312d2074b4: [Tunnelbroker] add MessageToDeviceRequestStatus message.

kamil mentioned this in D9521: [Tunnelbroker] improve Tunnelbroker messages docs.Oct 18 2023, 3:22 AM

kamil added inline comments.Oct 18 2023, 3:24 AM