Skip to content

Fix execution layer redundancy#5588

Merged
mergify[bot] merged 4 commits intosigp:unstablefrom
iamit-singh:fix-execution-layer-redundancy
Apr 18, 2024
Merged

Fix execution layer redundancy#5588
mergify[bot] merged 4 commits intosigp:unstablefrom
iamit-singh:fix-execution-layer-redundancy

Conversation

@iamit-singh
Copy link
Contributor

@iamit-singh iamit-singh commented Apr 16, 2024 •
edited
Loading

Issue Addressed

#3307

Proposed Changes

renamed and changed types(from Vec to Option) of below fields of Config struct:
execution_endpoints: Vec => execution_endpoint: Option
secret_files: Vec => secret_file: Option

Additional Info

@iamit-singh iamit-singh mentioned this pull request Apr 16, 2024
Closed
realbigsean
realbigsean previously approved these changes Apr 16, 2024
View reviewed changes
Copy link
Member

@realbigsean realbigsean left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! It's a nice cleanup, just had one small suggestion

@realbigsean realbigsean dismissed their stale review April 16, 2024 14:45

Whoops, looks like CI is failing

@realbigsean
Copy link
Member

realbigsean commented Apr 16, 2024

Looks like some tests need updating as well, if you want to check locally whether tests compile make lint will also check them

@realbigsean realbigsean added the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Apr 16, 2024
@iamit-singh
Copy link
Contributor Author

iamit-singh commented Apr 16, 2024

@realbigsean thanks for review. Actually I ran make lint in my local, strangely it doesn't give any error. Anyways, I see the errors in here, I'll work on them.

@realbigsean
Copy link
Member

realbigsean commented Apr 16, 2024

@realbigsean thanks for review. Actually I ran make lint in my local, strangely it doesn't give any error. Anyways, I see the errors in here, I'll work on them.

ah there must be some tests are are missed. usually i also run make test-beacon-chain and that seems to cover the rest

@iamit-singh
Copy link
Contributor Author

iamit-singh commented Apr 17, 2024

@realbigsean fixed the tests!

realbigsean
realbigsean approved these changes Apr 17, 2024
View reviewed changes
Copy link
Member

@realbigsean realbigsean left a comment

Choose a reason for hiding this comment

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

Awesome!

@realbigsean
Copy link
Member

realbigsean commented Apr 17, 2024

@mergify queue

@realbigsean realbigsean added ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Apr 17, 2024
@mergify
Copy link

mergify bot commented Apr 17, 2024 •
edited
Loading

queue

🛑 The pull request has been removed from the queue default

Details

The queue 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.

mergify bot added a commit that referenced this pull request Apr 17, 2024
@mergify
Merge of #5588
0a0cb6b
@mergify mergify bot mentioned this pull request Apr 17, 2024
Closed
5 tasks
@realbigsean
Copy link
Member

realbigsean commented Apr 17, 2024

@mergify requeue

@mergify
Copy link

mergify bot commented Apr 17, 2024

requeue

✅ This pull request will be re-embarked automatically

Details

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

@mergify
Copy link

mergify bot commented Apr 17, 2024 •
edited
Loading

queue

🛑 The pull request has been removed from the queue default

Details

The queue 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.

mergify bot added a commit that referenced this pull request Apr 17, 2024
@mergify
Merge of #5588
0588692
@mergify mergify bot mentioned this pull request Apr 17, 2024
Closed
5 tasks
@michaelsproul
Copy link
Member

michaelsproul commented Apr 18, 2024

@Mergifyio requeue

@mergify
Copy link

mergify bot commented Apr 18, 2024

requeue

✅ This pull request will be re-embarked automatically

Details

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

@mergify
Copy link

mergify bot commented Apr 18, 2024 •
edited
Loading

queue

✅ The pull request has been merged automatically

Details

The pull request has been merged automatically at 62e4abf

mergify bot added a commit that referenced this pull request Apr 18, 2024
@mergify
Merge of #5588
73e7346
This was referenced Apr 18, 2024
Closed
Merged
@mergify mergify bot merged commit 62e4abf into sigp:unstable Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@realbigsean realbigsean realbigsean approved these changes

Assignees

No one assigned

Labels

ready-for-merge This PR is ready to merge.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@iamit-singh @realbigsean @michaelsproul