Skip to content

chore: update style guidelines#2158

Merged
SamWilsn merged 2 commits intoethereum:forks/amsterdamfrom
SamWilsn:update-contributing
Feb 11, 2026
Merged

chore: update style guidelines#2158
SamWilsn merged 2 commits intoethereum:forks/amsterdamfrom
SamWilsn:update-contributing

Conversation

@SamWilsn
Copy link
Contributor

@SamWilsn SamWilsn commented Feb 6, 2026

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

Copy link
Contributor

@fselmo fselmo left a comment

Choose a reason for hiding this comment

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

lgtm, minor spelling correction but approving to unblock once that's in

# EIP-1234: The dingus is the rate of fleep
dingus = a + b

# EIP-4567: Frobulate the dingus
Copy link
Contributor

Choose a reason for hiding this comment

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

😆

Copy link
Member

@danceratopz danceratopz left a comment

Choose a reason for hiding this comment

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

Looks good @SamWilsn! Just one question below regarding docstrings.

The marked lines (`<-`) are now incorrectly attributed to EIP-4567 in Fork+1. Instead, omit the EIP identifier in the comments, and describe the changes introduced by the EIP in the function's docstrings. The rendered diffs will make it pretty obvious what's changed.
</details>

#### Docstrings
Copy link
Member

Choose a reason for hiding this comment

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

Sam, was a standard ever set for EELS docstrings to be either declarative (numpy) or imperative (PEP 257). I spotted that we currently use both, perhaps we can try to stick to one going forward?

For example:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good question. I have no idea.

Personally, my top priority is eventually getting to something like the CL annotated spec. I'm not sure the various projects attempting to do that agree on much though.

RazorClient pushed a commit to RazorClient/execution-specs that referenced this pull request Feb 11, 2026
Co-authored-by: felipe <fselmo2@gmail.com>
@SamWilsn SamWilsn merged commit 44b328d into ethereum:forks/amsterdam Feb 11, 2026
4 checks passed
@SamWilsn SamWilsn deleted the update-contributing branch February 11, 2026 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants