8000 fix(indexer): Ineffective or missing break statements in kv package. by alesforz · Pull Request #3557 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(indexer): Ineffective or missing break statements in kv package. #3557

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

Merged
merged 4 commits into from
Jul 26, 2024

Conversation

alesforz
Copy link
Contributor
@alesforz alesforz commented Jul 25, 2024

Closes #3544.

Changes

  • Adds missing for loop labels in the kv indexer
  • Previously ineffective break statements now point to their enclosing for loop labels to exit upon reception on the ctx.Done() channel.

PR checklist

- [ ] Tests written/updated

  • Changelog entry added in .changelog (we use unclog to manage our changelog)
    - [ ] Updated relevant documentation (docs/ or spec/) and code comments
  • Title follows the Conventional Commits spec

@alesforz alesforz added bug Something isn't working indexer labels Jul 25, 2024
@alesforz alesforz self-assigned this Jul 25, 2024
@alesforz alesforz linked an issue Jul 25, 2024 that may be closed by this pull request
@alesforz alesforz force-pushed the 3544-indexer-break-statement branch from c69acd5 to 471ec5e Compare July 25, 2024 14:39
@alesforz alesforz added backport-to-v0.37.x Tell Mergify to backport the PR to v0.37.x backport-to-v0.38.x Tell Mergify to backport the PR to v0.38.x backport-to-v1.x Tell Mergify to backport the PR to v1.x labels Jul 25, 2024
@alesforz alesforz marked this pull request as ready for review July 25, 2024 14:55
@alesforz alesforz requested review from a team as code owners July 25, 2024 14:55
Copy link
Contributor
@sergio-mena sergio-mena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The breaks you've modified LGTM. The breaks you've introduced also LGTM, but I have less confidence on my review there.
I'd wait for @jmalicevic to take a quick look at those introduced break statements.

Co-authored-by: Sergio Mena <sergio@informal.systems>
Copy link
Contributor
@melekes melekes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@alesforz alesforz added this pull request to the merge queue Jul 26, 2024
Merged via the queue into main with commit 79da036 Jul 26, 2024
39 checks passed
@alesforz alesforz deleted the 3544-indexer-break-statement branch July 26, 2024 07:50
mergify bot pushed a commit that referenced this pull request Jul 26, 2024
…3557)

Closes #3544.

### Changes
- Adds missing `for` loop labels in the kv indexer
- Previously ineffective `break` statements now point to their enclosing
`for` loop labels to exit upon reception on the `ctx.Done()` channel.

---

#### PR checklist

~- [ ] Tests written/updated~
- [x] 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

---------

Co-authored-by: Sergio Mena <sergio@informal.systems>
(cherry picked from commit 79da036)

# Conflicts:
#	state/indexer/block/kv/kv.go
mergify bot pushed a commit that referenced this pull request Jul 26, 2024
…3557)

Closes #3544.

### Changes
- Adds missing `for` loop labels in the kv indexer
- Previously ineffective `break` statements now point to their enclosing
`for` loop labels to exit upon reception on the `ctx.Done()` channel.

---

#### PR checklist

~- [ ] Tests written/updated~
- [x] 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

---------

Co-authored-
8000
by: Sergio Mena <sergio@informal.systems>
(cherry picked from commit 79da036)

# Conflicts:
#	state/indexer/block/kv/kv.go
mergify bot pushed a commit that referenced this pull request Jul 26, 2024
…3557)

Closes #3544.

### Changes
- Adds missing `for` loop labels in the kv indexer
- Previously ineffective `break` statements now point to their enclosing
`for` loop labels to exit upon reception on the `ctx.Done()` channel.

---

#### PR checklist

~- [ ] Tests written/updated~
- [x] 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

---------

Co-authored-by: Sergio Mena <sergio@informal.systems>
(cherry picked from commit 79da036)
alesforz added a commit that referenced this pull request Jul 26, 2024
…(backport #3557) (#3562)

Closes #3544.

### Changes
- Adds missing `for` loop labels in the kv indexer 
- Previously ineffective `break` statements now point to their enclosing
`for` loop labels to exit upon reception on the `ctx.Done()` channel.

---

#### PR checklist

~- [ ] Tests written/updated~
- [x] 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
<hr>This is an automatic backport of pull request #3557 done by
[Mergify](https://mergify.com).

Co-authored-by: Alessandro Sforzin <alessandro@informal.systems>
alesforz added a commit that referenced this pull request Jul 26, 2024
…(backport #3557) (#3564)

Closes #3544.

### Changes
- Adds missing `for` loop labels in the kv indexer 
- Previously ineffective `break` statements now point to their enclosing
`for` loop labels to exit upon reception on the `ctx.Done()` channel.

---

#### PR checklist

~- [ ] Tests written/updated~
- [x] 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
<hr>This is an automatic backport of pull request #3557 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Alessandro <alessandro@informal.systems>
alesforz added a commit that referenced this pull request Jul 26, 2024
…(backport #3557) (#3563)

Closes #3544.

### Changes
- Adds missing `for` loop labels in the kv indexer 
- Previously ineffective `break` statements now point to their enclosing
`for` loop labels to exit upon reception on the `ctx.Done()` channel.

---

#### PR checklist

~- [ ] Tests written/updated~
- [x] 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
<hr>This is an automatic backport of pull request #3557 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Alessandro <alessandro@informal.systems>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-v0.37.x Tell Mergify to backport the PR to v0.37.x backport-to-v0.38.x Tell Mergify to backport the PR to v0.38.x backport-to-v1.x Tell Mergify to backport the PR to v1.x bug Something isn't working indexer
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

fix(indexer): Ineffective or missing break statements in kv package.
3 participants
0