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

Move sync active requests to own modules #6272

Merged
merged 2 commits into from
Sep 12, 2024

Conversation

dapplion
Copy link
Collaborator

@dapplion dapplion commented Aug 16, 2024

Issue Addressed

PeerDAS will add two more sync ActiveRequest*. Currently there are only two: ActiveBlocksByRootRequest, ActiveBlobsByRootRequest as spaghetti in the same file

Proposed Changes

Move each ActiveRequest* to its own file to make space for upcoming PeerDAS PR with additional requests.

No change in logic or code, just copy pasted into new files

@dapplion dapplion added the ready-for-review The code is ready for review label Aug 16, 2024
@dapplion
Copy link
Collaborator Author

dapplion commented Sep 4, 2024

Fixed conflicts 👍

Copy link
Member

@AgeManning AgeManning left a comment

Choose a reason for hiding this comment

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

lgtm

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Sep 9, 2024
@jimmygchen
Copy link
Member

@mergify queue

Copy link

mergify bot commented Sep 9, 2024

queue

🛑 The pull request has been removed from the queue default

Pull request #6272 has been dequeued by a dequeue command.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@jimmygchen
Copy link
Member

@mergify dequeue

Copy link

mergify bot commented Sep 9, 2024

dequeue

✅ The pull request has been removed from the queue default

@jimmygchen
Copy link
Member

@mergify queue

Copy link

mergify bot commented Sep 9, 2024

queue

🛑 The pull request has been removed from the queue default

Pull request #6272 has been dequeued by a dequeue command.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@jimmygchen
Copy link
Member

@mergify requeue

Copy link

mergify bot commented Sep 9, 2024

requeue

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request

Copy link

mergify bot commented Sep 9, 2024

queue

🛑 The pull request has been removed from the queue default

The merge conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@jimmygchen
Copy link
Member

@mergify requeue

Copy link

mergify bot commented Sep 12, 2024

requeue

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request

Copy link

mergify bot commented Sep 12, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at e0ccadb

mergify bot added a commit that referenced this pull request Sep 12, 2024
@mergify mergify bot mentioned this pull request Sep 12, 2024
@mergify mergify bot merged commit e0ccadb into sigp:unstable Sep 12, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants