Conversation
|
Please note that this PR illustrates the flaky test problem. |
This reverts commit cac9e06.
86499ed to
06e8c90
Compare
|
Let's run the p2p tests and profile them, to learn how much RAM we can save |
|
|
|
@faddat what is left here? thinking of closing this since it seems very delicate and low confidence in changes |
|
Low confidence in the changes or in the code that tests them? |
|
@adizere my thoughts on closing this or not -
Could there be a problem in my changes? Yep, code can have bugs Is there an issue with test flakiness in this repo? Yep Do flaky tests make it impossible (at times) or difficult (at other times) to tell the difference between the two? yep From my POV, this is a worthy concept of a PR, and it could be that it is not ideally organized, and could need to be rewritten or passed over again. But it is significantly harder to know because I also know that the tests overall aren't reliable. If you'd like a rewrite though, please just let me know that, along with some feedback. Happy to do so. |
|
I'm going to review this tomorrow. |
| "vote_extensions_disabled", voteExtensionsDisabled, | ||
| "blocks_synced", blocksSynced, | ||
| "initial_commit_has_extensions", initialCommitHasExtensions, | ||
| ) |
There was a problem hiding this comment.
return is missing here!
in the original code:
402: continue FOR_LOOP
|
Closing in favor of #2820 |
Replaces #2152 Co-authored by: @faddat --- #### PR checklist - [ ] ~~Tests written/updated~~ - [ ] ~~Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog)~~ - [ ] ~~Updated relevant documentation (`docs/` or `spec/`) and code comments~~ - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
By various metrics, poolRoutine is the most complex function in CometBFT.
This PR is from #2130, which I'll break down into smaller PRs similar to the flaky test PR. I was able to identify this function as excessively complex by configuring linters in the way described in #2130.
review notes
I ended up removing some comments from the code, but it's possible that they should have been moved to one of the new helper functions. Please let me know if the comments should be restored.
PR checklist
.changelog(we use unclog to manage our changelog)docs/orspec/) and code comments