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

Fix execution layer redundancy #5588

Merged
merged 4 commits into from
Apr 18, 2024

Conversation

iamit-singh
Copy link
Contributor

@iamit-singh iamit-singh commented Apr 16, 2024

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

realbigsean
realbigsean previously approved these changes Apr 16, 2024
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

beacon_node/beacon_chain/src/test_utils.rs Outdated Show resolved Hide resolved
@realbigsean realbigsean dismissed their stale review April 16, 2024 14:45

Whoops, looks like CI is failing

@realbigsean
Copy link
Member

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

@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 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

@realbigsean fixed the tests!

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

@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
Copy link

mergify bot commented Apr 17, 2024

queue

🛑 The pull request has been removed from the queue default

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
@realbigsean
Copy link
Member

@mergify requeue

Copy link

mergify bot commented Apr 17, 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 Apr 17, 2024

queue

🛑 The pull request has been removed from the queue default

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
@michaelsproul
Copy link
Member

@Mergifyio requeue

Copy link

mergify bot commented Apr 18, 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 Apr 18, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 62e4abf

@mergify mergify bot merged commit 62e4abf into sigp:unstable Apr 18, 2024
29 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