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.

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

refactor: params.ChainConfigExtra & RulesExtra -> params/extras #702

Merged
merged 19 commits into from
Dec 18, 2024

Conversation

darioush
Copy link
Collaborator

@darioush darioush commented Dec 11, 2024

Moves the ChainConfigExtra type to a new package which doesn't register libevm type configurations.

params/rules_extra.go Outdated Show resolved Hide resolved
params/extras/config_extra.go Outdated Show resolved Hide resolved
params/extras/config_extra.go Outdated Show resolved Hide resolved
params/extras/config_extra.go Outdated Show resolved Hide resolved
params/rules_extra.go Outdated Show resolved Hide resolved
@darioush darioush changed the title refactor: params.ChainConfigExtra -> params/extras refactor: params.ChainConfigExtra & RulesExtra -> params/extras Dec 17, 2024
@darioush darioush marked this pull request as ready for review December 17, 2024 20:43
@darioush darioush requested review from ceyonur and a team as code owners December 17, 2024 20:43
@darioush darioush requested a review from ARR4N December 17, 2024 22:38
Copy link
Collaborator

@qdm12 qdm12 left a comment

Choose a reason for hiding this comment

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

Good job 👍

All my comments are on a small things to change in existing code that got simply moved to params/extras, but which shows as "new code" by git (git doesn't detect the file movement); I prefixed these comments with OOS/part-of-new-git-deltas: to highlight I'm aware this is not new new code.

How is our team feeling about this: are we ok to improve/fix small things when we have code showing as new git additions, despite it being out of scope with the PR (as long as it's not geth specific code)? I personally find this as a good way to improve code/comments occasionally, especially since we will likely not focus on reviewing all the code otherwise, but happy to not do this in the future if we feel it hurts us more than it helps us.

params/extras/config.go Show resolved Hide resolved
params/extras/config.go Outdated Show resolved Hide resolved
params/extras/config.go Outdated Show resolved Hide resolved
params/extras/config.go Show resolved Hide resolved
params/extras/config.go Show resolved Hide resolved
params/extras/config.go Outdated Show resolved Hide resolved
params/extras/config.go Show resolved Hide resolved
params/extras/config.go Show resolved Hide resolved
params/extras/config.go Outdated Show resolved Hide resolved
params/extras/config.go Outdated Show resolved Hide resolved
params/extras/config.go Outdated Show resolved Hide resolved
params/hooks_libevm.go Show resolved Hide resolved
@darioush darioush enabled auto-merge (squash) December 18, 2024 17:59
@darioush darioush merged commit a7f7061 into libevm Dec 18, 2024
6 checks passed
@darioush darioush deleted the params-extra-refactor branch December 18, 2024 18:00
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