-
Notifications
You must be signed in to change notification settings - Fork 684
v0.37.x: backport of new content on spec/p2p #1004
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
Conversation
p2p/README.md
Outdated
- [Connection](https://github.com/cometbft/cometbft/blob/v0.37.x/spec/p2p/legacy-docs/connection.md) for details on how connections and multiplexing work | ||
- [Peer](https://github.com/cometbft/cometbft/blob/v0.37.x/spec/p2p/legacy-docs/node.md) for details on peer ID, handshakes, and peer exchange | ||
- [Node](https://github.com/cometbft/cometbft/blob/v0.37.x/spec/p2p/legacy-docs/node.md) for details about different types of nodes and how they should work | ||
- [Config](https://github.com/cometbft/cometbft/blob/v0.37.x/spec/p2p/legacy-docs/config.md) for details on some config option |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These must be relative links if you want this rendered correctly on the docs site.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure? This is on p2p/README.md
, not part of the docs or spec dirs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, you're right. This wouldn't affect the docs then. But it's still a hard link to the repo as opposed to a relative link to a file that exists within the same repo - is that intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have seen some hard links on source code directories, there are several actually. I don't really like this idea, to be honest.
Anyway, I will fix all the links in the files involved in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed full links on 06d0804
For reviewers, the changes to have in mind for this backport PR are few, the following (remove
Notice that most of the diff under |
Any blocker for merging this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I skimmed through the changes and they LGTM.
Just one note for next time:
The command
git diff --stat main -- spec/p2p/ spec/abci/ p2p/README.md
is actually showing noise: changes that are not really contained in this PR (I got confused by it). Best method is just to backport, 1 by 1, the PRs on main
. This might seem more tedious, but makes the reviewer's life much easier (in that the same changes don't need to be reviewed twice, whilst having confidence we're not missing anything to be reviewed).
Thank you @sergio-mena. I had the same thoughts after this attempt, what is more appropriate: multiple backports PRs or a single multiple-PR backport? I planned to pose this question to the team, but in general I agree with yout that backporting PRs one by one is more bureaucratic but probably more appropriate than opting for a multiple-PR backport. |
* spec/p2p: starting the specification of a Reactor * spec/p2p: generic service modelled in Quint * spec/p2p: minimal/initial reactor model in Quint * spec/p2p: P2P -> reactor interaction grammar * spec/p2p: routines modeled in reactor.qnt * spec/p2p: reactor.Receive) modeled in Quint * spec/p2p: reactor types moved to the beginning * spec/p2p: reactor Quint state encapsulated * spec/p2p: reactor with actions and defintions * spec/p2p: reactor SetSwitch() method modeled * spec/p2p: reactor Service interface modeled * spec/p2p: reactor setup action added * spec/p2p: reactor Quint spec cleanup * spec/p2p: reactor InitPeer spec updated * spec/p2p: reactor Quint simulation support * spec/p2p: reactor Quint spec refactored * spec/p2p: reactor grammar added to a rephrased README * spec/p2p: README structure reorganized * spec/p2p: reactor registration, overview * spec/p2p: reactor implements a service (start/stop) * spec/p2p: reactor peer management, overview * spec/p2p: reactor receive message method documented * spec/p2p: renamed to registration/register reactor * spec/p2p: reactor grammar refactoring, part I * spec/p2p: reactor grammar refactoring, part II * spec/p2p: reactor grammar refactoring, part III * spec/p2p: reactor grammar refactoring, part IV * spec/p2p: removing Quint model used as an example * spec/p2p: reactor grammar refactoring, part V * spec/p2p: reactor grammar refactoring, part VI * spec/p2p: reactor grammar refactoring, part VII * Apply suggestions from code review Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com> * spec/p2p: reactor Quint model, some comments added * Update spec/p2p/reactor/reactor.qnt Co-authored-by: Lasaro <lasaro@informal.systems> * spec/p2p: syntax HL for Quint files Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com> * spec/p2p: suggestions from code review Co-authored-by: Sergio Mena <sergio@informal.systems> * spec/p2p: reactor Quint model, channel ID as str * spec/p2p: applying suggestions from code review * spec/p2p: API methods first, then actions on Quint * spec/p2p: reactor Quint model, minor code cleanup * spec/p2p: reactor README intro slightly rephrased * spec/p2p: reactor grammar intro slightly rephrased * spec/p2p: some reactor "should" replaced by "must" * spec/p2p: Quint model reference on README updated * spec/p2p: minor fixes on reactor's README --------- Co-authored-by: Daniel Cason <cason@lausanne.local> Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com> Co-authored-by: Lasaro <lasaro@informal.systems> Co-authored-by: Sergio Mena <sergio@informal.systems>
* spec/p2p: registered flag in Quint reactor * spec/p2p: NewReactor method in Quint reactor * spec/p2p: reactor is registered with a name * spec/p2p: reactor actions receive ReactorState * spec/p2p: Quint model supports multiple reactors * spec/p2p: Quint reactor/service state simplified * spec/p2p: Quint reactor has channels' list as field * spec/p2p: Quint updateReactorTo action helper * spec/p2p: reactors are assigned to channel IDs * spec/p2p: reactor API specification, small fixes * spec/p2p: API p2p layer provides to reactors * spec/p2p: reactor API content to reactor.md file * spec/p2p: new README for reactors with a diagram * spec/p2p: API for reactors, introduction text * spec/p2p: links between the two API documentations * spec/p2p: suggestions from code review Co-authored-by: Sergio Mena <sergio@informal.systems> * spec/p2p: Apply suggestions from code review Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com> Co-authored-by: Lasaro <lasaro@informal.systems> * spec/p2p: reactor documentation intro, small fix * spec/p2p: a paragraph about concurrency and reactors * spec/p2p: intro to the reactor grammar, minor add * spec/p2p: notes about switch broadcast method * spec/p2p: note on NumPeers() switch method * spec/p2p: reactor veting peers, some rephrasing * spec/p2p: link to discussion on peer quality * spec/p2p: link to stop peer discussion on other repo * spec/p2p: link to state sharing between reactors added * spec/p2p: link to reactor API split added --------- Co-authored-by: Sergio Mena <sergio@informal.systems> Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com> Co-authored-by: Lasaro <lasaro@informal.systems>
* spec/p2p: moving legacy documents to legacy/ dir * spec/p2p: using capitalized names for README.md * spec/p2p: intro to p2p spec, moved from other README * spec/p2p: v0.34.x documentation, README shortened * spec/p2p: minor changes on READMEs * spec/p2p: moving messages/ content to legacy/ dir * spec/p2p: dir for docs of the p2p implementation * spec/p2p: renamed reactor dirs to reactor-api * spec/p2p: legacy content to legacy-docs/ dir * spec/p2p: fixes on implementation/README.md file * spec/p2p: moving images to a single subdir * spec/p2p: fixing Markdown links * spec/p2p: fixing Markdown links in all repo * spec/p2p: table of contents for the p2p docs * spec/p2p: applying suggestions from Josef Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com> --------- Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com>
06d0804
to
b03a6d1
Compare
My impression is that 1:1 backports, as they are more mechanical, they are easier to review (e.g. if you use diff-of-diffs for reviewing backports it's a 5 min task... and if you let mergify do it and there are no conflicts, it's a 0 min task). But, since you're asking formally, I'm 100% convinced I'm right. What I propose: next time it happens, ping me and we look at it together. |
This is backport of three PRs originally merged against main: * spec/p2p: Specify the operation of a Reactor (cometbft#714) * spec/p2p: document the p2p API used by Reactors (cometbft#851) * spec/p2p: new structure for the p2p specification (cometbft#966) Co-authored-by: Josef Widder <44643235+josef-widder@users.noreply.github.com> Co-authored-by: Lasaro <lasaro@informal.systems> Co-authored-by: Sergio Mena <sergio@informal.systems>
This PR backports 3 PRs merged into the
main
branch: #714, #851, and #966.Note to reviewers: there is no new content added here, this is just a manual backport of multiple PRs already merged into main.
Addresses #598