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

Update Forkdiff #214

Merged
merged 4 commits into from
Aug 24, 2024
Merged

Update Forkdiff #214

merged 4 commits into from
Aug 24, 2024

Conversation

mininny
Copy link
Member

@mininny mininny commented Aug 11, 2024

@mininny mininny force-pushed the feature/mininny/update-forkdiff branch from a3be9ad to c95189e Compare August 11, 2024 07:48
Copy link
Member

@pcw109550 pcw109550 left a comment

Choose a reason for hiding this comment

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

List of suggestions

  • Can you remove erigon-lib hyperlink because it is not used anymore?
  • Also ignore diffs for core/allocs/optimism_devnet.json because this diff adds +15277 line diffs but only needed for genesis allocs.

@mininny
Copy link
Member Author

mininny commented Aug 17, 2024

List of suggestions

  • Can you remove erigon-lib hyperlink because it is not used anymore?
  • Also ignore diffs for core/allocs/optimism_devnet.json because this diff adds +15277 line diffs but only needed for genesis allocs.

@pcw109550 Thanks for the suggestions! I removed erigon-lib and ignored the diffs for the .json file.
What about _test.go files? What do you think about moving them all to the "Testing" section and have them ignored since they increase the diff so much? https://op-geth.optimism.io/ geth also has most of the tests in the Testing section as well.

@pcw109550
Copy link
Member

@mininny ignoring test files will make forkdiff more consistent with op-geth forkdiff so i totally agree with that idea.

@mininny
Copy link
Member Author

mininny commented Aug 22, 2024

@mininny ignoring test files will make forkdiff more consistent with op-geth forkdiff so i totally agree with that idea.

It's updated! :)

@mininny mininny merged commit 1cd6a05 into op-erigon Aug 24, 2024
4 checks passed
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