Skip to content

Auto manage SpecBuilder prev fork inheritance#3436

Merged
hwwhww merged 2 commits intoethereum:devfrom
dapplion:auto-spec-builder-dependency
Jun 26, 2023
Merged

Auto manage SpecBuilder prev fork inheritance#3436
hwwhww merged 2 commits intoethereum:devfrom
dapplion:auto-spec-builder-dependency

Conversation

@dapplion
Copy link
Member

Setting PREVIOUS_FORK_OF to FORK_B, but then inheriting from the SpecBuilder of FORK_C downstream issues that are hard to debug except for experienced mantainers.

This PR reduces a potential footgun by computing the inheritance of SpecBuilder objects automatically from the fork relationships specified in PREVIOUS_FORK_OF

return spec
ssz_dep_constants_verification,
]
return "\n\n\n".join([str.strip("\n") for str in spec_strs if str]) + "\n"
Copy link
Member Author

Choose a reason for hiding this comment

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

Because of the automated nature of computing the spec parts, I needed a way to ensure a constant 2 blank lines separation between each section

Copy link
Contributor

Choose a reason for hiding this comment

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

I love this clean up <3

Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

@dapplion I like it!
Indeed, maintaining auto-mergeable forks seems more straightforward than managing inheritance spec builders. 👍

return spec
ssz_dep_constants_verification,
]
return "\n\n\n".join([str.strip("\n") for str in spec_strs if str]) + "\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

I love this clean up <3

@hwwhww hwwhww added the testing CI, actions, tests, testing infra label Jun 26, 2023
@hwwhww hwwhww merged commit cc021de into ethereum:dev Jun 26, 2023
@dapplion dapplion deleted the auto-spec-builder-dependency branch June 26, 2023 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing CI, actions, tests, testing infra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants