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.

Sign up for GitHub

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

Jump to bottom

Fix Rust beta compiler errors (1.77) #5180

Merged
merged 7 commits into from
Feb 5, 2024
Merged

Fix Rust beta compiler errors (1.77) #5180

merged 7 commits into from
Feb 5, 2024

Conversation

jimmygchen
Copy link
Member

@jimmygchen jimmygchen commented Feb 5, 2024 •
edited
Loading

Issue Addressed

Fix compilation/lint errors when compiling lighthouse using 1.77 beta compiler.

The fixes for unused code aren't ideal, but I can't think of a better way rn (related issue here):

#[derive(Debug)]
// We don't use the inner values directly, but they're used in the Debug impl.
#[allow(dead_code)]
enum AttestationPerformanceError {
    BlockReplay(BlockReplayError),
    BeaconState(BeaconStateError),
    ParticipationCache(ParticipationCacheError),
    UnableToFindValidator(usize),
}

@jimmygchen jimmygchen marked this pull request as ready for review February 5, 2024 01:35
@jimmygchen jimmygchen added the ready-for-review The code is ready for review label Feb 5, 2024
@jimmygchen jimmygchen mentioned this pull request Feb 5, 2024
Merged
jxs
jxs approved these changes Feb 5, 2024
View reviewed changes
Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

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

LGTM Jimmy

beacon_node/execution_layer/src/test_utils/mod.rs Outdated Show resolved Hide resolved
beacon_node/lighthouse_network/tests/common.rs
Comment on lines +45 to +50
pub struct Libp2pInstance(
LibP2PService<ReqId, E>,
#[allow(dead_code)]
// This field is managed for lifetime purposes may not be used directly, hence the `#[allow(dead_code)]` attribute.
exit_future::Signal,
);
Copy link
Member

Choose a reason for hiding this comment

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

NB: submitted #5183, we don't need exit_future

Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking we merge this one first to get CI working and update this in #5183

@realbigsean
Copy link
Member

realbigsean commented Feb 5, 2024 •
edited
Loading

@Mergifyio queue

@mergify Mergify
Copy link

mergify bot commented Feb 5, 2024 •
edited
Loading

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 39e9f7d

@mergify mergify bot merged commit 39e9f7d into sigp:unstable Feb 5, 2024
29 checks passed
danielrachi1 pushed a commit to danielrachi1/lighthouse that referenced this pull request Feb 14, 2024
@jimmygchen @danielrachi1
Fix Rust beta compiler errors (1.77) (sigp#5180)
fceb6d8 
* Lint fixes

* More fixes for beta compiler.

* Format fixes

* Move `#[allow(dead_code)]` to field level.

* Remove old comment.

* Update beacon_node/execution_layer/src/test_utils/mod.rs

Co-authored-by: João Oliveira <[email protected]>

* remove duplicate line
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelsproul michaelsproul michaelsproul left review comments

@realbigsean realbigsean realbigsean left review comments

@jxs jxs jxs approved these changes

Assignees
No one assigned
Labels
ready-for-review The code is ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jimmygchen @realbigsean @jxs @michaelsproul