Skip to content

Conversation

chong-he
Copy link
Member

@chong-he chong-he commented Jul 21, 2025

Issue Addressed

Thanks @dknopik and @michaelsproul for your help!

@chong-he chong-he requested a review from michaelsproul as a code owner July 21, 2025 06:55
@chong-he chong-he added work-in-progress PR is a work-in-progress code-quality labels Jul 21, 2025
@chong-he chong-he requested a review from jxs as a code owner July 21, 2025 07:38
@mergify mergify bot removed the ready-for-review The code is ready for review label Aug 8, 2025
@chong-he
Copy link
Member Author

chong-he commented Aug 8, 2025

It looks like Rust 1.89 is in effect and is already linting the chain if / if let loop: https://github.com/sigp/lighthouse/actions/runs/16826607902/job/47664447929

I revised all in this single commit: 8665e63, unfortunately this makes the diff and files changed increased by a lot

@chong-he chong-he requested a review from macladson August 8, 2025 12:01
@chong-he chong-he added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Aug 8, 2025
@michaelsproul michaelsproul added the v8.0.0-rc.0 Q3 2025 release for Fusaka on Holesky label Aug 12, 2025
Copy link

mergify bot commented Aug 12, 2025

This pull request has merge conflicts. Could you please resolve them @chong-he? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Aug 12, 2025
@michaelsproul
Copy link
Member

I think we should merge this ASAP and include it in v8.0.0-rc.0, so that it doesn't come into conflict with any other refactors.

@macladson you probably have the best ability to review quickly as you can review just the diff since your last review.

@chong-he there is a single conflict now which would be good to fix so we can merge immediately once Mac gives the 👍

@chong-he chong-he added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Aug 12, 2025
Copy link
Member

@macladson macladson left a comment

Choose a reason for hiding this comment

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

Ok I've gone through all the new changes and it all looks good to me 👍
Nice work @chong-he!

Some of the new && chaining reads a bit awkwardly in some places but I think overall the 2024 changes are an upgrade

@macladson macladson added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Aug 12, 2025
Copy link

mergify bot commented Aug 12, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@chong-he
Copy link
Member Author

Ok I've gone through all the new changes and it all looks good to me 👍 Nice work @chong-he!

Some of the new && chaining reads a bit awkwardly in some places but I think overall the 2024 changes are an upgrade

Thank you so much for your detailed review!

@mergify mergify bot merged commit 522bd9e into sigp:unstable Aug 13, 2025
34 checks passed
@michaelsproul
Copy link
Member

Woohoo! Thanks Mac for the review and CK for the impl! ❤️

@chong-he chong-he deleted the edition2024 branch August 13, 2025 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-incompat Backwards-incompatible API change code-quality ready-for-merge This PR is ready to merge. v8.0.0-rc.0 Q3 2025 release for Fusaka on Holesky
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants