Skip to content

fix: use serde private mod patch notation - #2886

Merged
mattsse merged 1 commit into
alloy-rs:mainfrom
grandizzy:fix-serde
Sep 16, 2025
Merged

fix: use serde private mod patch notation#2886
mattsse merged 1 commit into
alloy-rs:mainfrom
grandizzy:fix-serde

Conversation

@grandizzy

@grandizzy grandizzy commented Sep 16, 2025

Copy link
Copy Markdown
Contributor

Motivation

Solution

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@grandizzy
grandizzy marked this pull request as ready for review September 16, 2025 07:05

@yash-atreya yash-atreya left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm!

context:

I think it is low cost and decent benefit. If there is any logic in there that someone wants access to, they need to find someone to maintain it officially so that their users do not experience repeated breakage from serde updates.

from serde-rs/serde#2980 (comment)

serde-rs/serde#2980 was released in a serde patch v1.0.225

@github-project-automation github-project-automation Bot moved this to Reviewed in Alloy Sep 16, 2025

@mattsse mattsse left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

let's get this out to get unblocked

@klkvr

I think we could also just remove the manual deserialize impl entirely?

https://github.com/alloy-rs/alloy/blob/main/crates/tx-macros/src/serde.rs#L228-L263

because I believe deriving the deserialize here would generate the same code but with a worse error message, although this would be preferred over having to maintain this

@mattsse
mattsse merged commit 372a631 into alloy-rs:main Sep 16, 2025
31 checks passed
@github-project-automation github-project-automation Bot moved this from Reviewed to Done in Alloy Sep 16, 2025
@grandizzy
grandizzy deleted the fix-serde branch September 16, 2025 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug] Build fails with serde version

3 participants