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.

Sign up for GitHub

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

Jump to bottom

fix(forkdiff): update sub definitions to fix hydration #244

Open
wants to merge 2 commits into
base: optimism
Choose a base branch
from
Open

fix(forkdiff): update sub definitions to fix hydration #244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3ca839d
fix(forkdiff): update sub definitions to fix hydration
sambacha Feb 13, 2024
97ec4ef
Merge branch 'optimism' into fix-forkdiff
sambacha Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .github/workflows/pages.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
name: Build and publish forkdiff github-pages
permissions:
contents: write

on:
push:
branches:
- optimism

jobs:
deploy:
concurrency: ci-${{ github.ref }}
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4
with:
fetch-depth: 1000 # make sure to fetch the old commit we diff against

Expand All @@ -23,7 +25,7 @@ jobs:
- name: Build pages
run: |
mkdir -p tmp/pages
mv index.html tmp/pages/index.html
mv index.html tmp/pages/index.html || { echo "Error: Failed to move index.html to tmp/pages/"; exit 1; }
touch tmp/pages/.nojekyll
if [ "$GITHUB_REPOSITORY" == "ethereum-optimism/op-geth" ]; then
echo "op-geth.optimism.io" > tmp/pages/CNAME
Expand Down
8 changes: 4 additions & 4 deletions fork.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
title: "op-geth - go-ethereum fork diff overview"
footer: |
Fork-diff overview of [`op-geth`](https://github.com/ethereum-optimism/op-geth), a fork of [`go-ethereum`](https://github.com/ethereum/go-ethereum).
Fork-diff overview of [`op-geth`](https://github.com/ethereum-optimism/op-geth), a fork of [`go-ethereum, v1.13.8`](https://github.com/ethereum/go-ethereum).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, we won't maintain manual versions in multiple places. Can you revert this part of the change?

and execution-engine of the [OP-stack](https://github.com/ethereum-optimism/optimism).
base:
name: go-ethereum
Expand All @@ -25,6 +25,8 @@ def:

- [L2 Execution Engine spec](https://github.com/ethereum-optimism/optimism/blob/develop/specs/exec-engine.md)
- [Deposit Transaction spec](https://github.com/ethereum-optimism/optimism/blob/develop/specs/deposits.md)

Last updated: Tue Feb 13 19:58:26 UTC 2024
Comment on lines +28 to +29
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Last updated: Tue Feb 13 19:58:26 UTC 2024
Last updated: Tue Feb 13 19:58:26 UTC 2024

We're 100% going to forget updating this. The commit hash in the diff page is the best source of truth for last udpate.

sub:
- title: "Core modifications"
sub:
Expand Down Expand Up @@ -119,7 +121,7 @@ def:
description: |
The rollup functionality is enabled with the `optimism` field in the chain config.
The EIP-1559 parameters are configurable to adjust for faster more frequent and smaller blocks.
The parameters can be overriden for testing.
The parameters can be overridden for testing.
globs:
- "params/config.go"
- "params/protocol_params.go"
Expand Down Expand Up @@ -166,7 +168,6 @@ def:
- "eth/ethconfig/config.go"
- title: Tx gossip disable option
globs:
- "eth/handler.go"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it spit out the error because of the usage of this diff in two places. We should add a description of the eth/handler.go changes in one place, to keep some documentation of the total set of changes.

- "eth/handler_eth.go"
- title: Warn on missing hardfork data
globs:
Expand All @@ -182,7 +183,6 @@ def:
- title: Historical data for Snap-sync
description: Snap-sync has access to trusted Deposit Transaction Nonce Data.
globs:
- "eth/handler.go"
- "eth/downloader/downloader.go"
- "eth/downloader/receiptreference.go"
- title: Discv5 node discovery
Expand Down