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

docs(state): Use different terms for block verification and state queues #7061

Merged
merged 10 commits into from
Jul 4, 2023

Conversation

oxarbitrage
Copy link
Contributor

Motivation

Close #6681

Solution

Do suggested renames.

Review

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

Follow Up Work

@oxarbitrage
Copy link
Contributor Author

This PR should be rebased to main after #7035 is merged.

I forgot how to set up an automatic rebase. I will add that when i find the feature.

@oxarbitrage oxarbitrage changed the base branch from main to add-FinalizableBlock June 25, 2023 21:30
@codecov
Copy link

codecov bot commented Jun 25, 2023

Codecov Report

Merging #7061 (abac8d9) into main (1f1d04b) will decrease coverage by 0.28%.
The diff coverage is 61.11%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7061      +/-   ##
==========================================
- Coverage   77.49%   77.22%   -0.28%     
==========================================
  Files         310      310              
  Lines       41716    41833     +117     
==========================================
- Hits        32329    32304      -25     
- Misses       9387     9529     +142     

Base automatically changed from add-FinalizableBlock to main June 27, 2023 08:58
@oxarbitrage oxarbitrage marked this pull request as ready for review June 28, 2023 13:23
@oxarbitrage oxarbitrage requested a review from a team as a code owner June 28, 2023 13:23
@oxarbitrage oxarbitrage requested review from teor2345 and removed request for a team June 28, 2023 13:23
teor2345
teor2345 previously approved these changes Jun 29, 2023
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for doing this work! I really appreciate it.

The old comments are very confusing, so I made some suggestions where we could make them clearer. I'm also going to ask @arya2 to review as well, because he's more familiar with some parts of this code.

@teor2345 teor2345 added do-not-merge Tells Mergify not to merge this PR and removed do-not-merge Tells Mergify not to merge this PR labels Jun 30, 2023
Co-authored-by: teor <teor@riseup.net>
Copy link
Contributor

@arya2 arya2 left a comment

Choose a reason for hiding this comment

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

This looks great, much clearer, thank you!

mergify bot added a commit that referenced this pull request Jul 4, 2023
@mergify mergify bot merged commit 5859fac into main Jul 4, 2023
@mergify mergify bot deleted the issue6681 branch July 4, 2023 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update documentation to clarify terminology around block verification
3 participants