Skip to content

fix(op-batcher): Fix PR #5186 Nits#5202

Merged
mergify[bot] merged 1 commit intodevelopfrom
refcell/batcher/cleanup
Mar 21, 2023
Merged

fix(op-batcher): Fix PR #5186 Nits#5202
mergify[bot] merged 1 commit intodevelopfrom
refcell/batcher/cleanup

Conversation

@refcell
Copy link
Contributor

@refcell refcell commented Mar 20, 2023

Description

Fixes CLI-3678

PR #5186 was eagerly merged without proper review + fixes. This PR implements those requested minor nits relating to the op-batcher's MaxFrameSize validation.

Note: #5187 introduces a FrameV0OverHeadSize constant in the op-node rollup that can be used in place of the constant introduced here. As such, probably makes sense to wait for #5187 to be merged and rebase this pr.

@refcell refcell requested a review from a team as a code owner March 20, 2023 16:12
@refcell refcell requested a review from tynes March 20, 2023 16:12
@changeset-bot
Copy link

changeset-bot bot commented Mar 20, 2023

⚠️ No Changeset found

Latest commit: 25c7243

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@refcell refcell self-assigned this Mar 20, 2023
@netlify
Copy link

netlify bot commented Mar 20, 2023

Deploy Preview for opstack-docs canceled.

Name Link
🔨 Latest commit 25c7243
🔍 Latest deploy log https://app.netlify.com/sites/opstack-docs/deploys/6419e7c4dc8c9e000855c53e

@refcell refcell requested review from sebastianst and trianglesphere and removed request for tynes March 20, 2023 16:12
@codecov
Copy link

codecov bot commented Mar 20, 2023

Codecov Report

Merging #5202 (946fdb3) into develop (0aab6a5) will decrease coverage by 0.12%.
The diff coverage is 100.00%.

❗ Current head 946fdb3 differs from pull request most recent head 25c7243. Consider uploading reports for the commit 25c7243 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #5202      +/-   ##
===========================================
- Coverage    37.67%   37.56%   -0.12%     
===========================================
  Files          215      216       +1     
  Lines        18249    18192      -57     
===========================================
- Hits          6876     6834      -42     
+ Misses       10699    10689      -10     
+ Partials       674      669       -5     
Flag Coverage Δ
bedrock-go-tests 37.56% <100.00%> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
op-batcher/batcher/channel_builder.go 82.58% <100.00%> (ø)

... and 13 files with indirect coverage changes

Copy link
Member

@sebastianst sebastianst left a comment

Choose a reason for hiding this comment

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

LGTM but shouldn't be merged yet until FrameV0OverHeadSize is imported.

@sebastianst
Copy link
Member

can be rebased now.

@refcell refcell requested review from a team and protolambda as code owners March 21, 2023 17:11
@refcell refcell requested a review from smartcontracts March 21, 2023 17:11
@refcell refcell force-pushed the refcell/batcher/cleanup branch from 5ffe6e3 to 4a6c53b Compare March 21, 2023 17:19
@refcell refcell force-pushed the refcell/batcher/cleanup branch from 4a6c53b to 25c7243 Compare March 21, 2023 17:22
@refcell refcell requested a review from sebastianst March 21, 2023 17:22
Copy link
Member

@sebastianst sebastianst left a comment

Choose a reason for hiding this comment

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

beautiful ✨

@mergify mergify bot merged commit 3d915fa into develop Mar 21, 2023
@mergify mergify bot deleted the refcell/batcher/cleanup branch March 21, 2023 17:42
@mergify
Copy link
Contributor

mergify bot commented Mar 21, 2023

This PR has been added to the merge queue, and will be merged soon.

@mergify
Copy link
Contributor

mergify bot commented Mar 21, 2023

This PR is next in line to be merged, and will be merged as soon as checks pass.

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.

4 participants