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

forkdiff update #321

Merged
merged 2 commits into from
May 23, 2024
Merged

forkdiff update #321

merged 2 commits into from
May 23, 2024

Conversation

protolambda
Copy link
Contributor

Description

I updated my forkdiff tool to:

  • Support ** glob pattern (Go by default doesn't support it)
  • Less strict with file-matching: if a later pattern matches a previous diff, but if that diff was already rendered, then don't render it again, but don't error like before either. This makes fallback glob patterns a lot easier.
  • Support non-global ignored matching, useful to group tests with their functions, without counting them towards the final total diff line count.
  • Now listing the ignored added/deleted lines separately, instead of completely ignoring.

Updated fork.yaml to:

  • use the new type of ignored diff
  • fix reference of upstream geth, there should only be a commit hash
  • improve/fix a few extending patterns, to capture their contents properly
  • add a few data files to the globally ignored section
  • catch-all for tests, to not pollute the "other changes" section

The docker forkdiff is already updated, CI should pick up the changes.

Expected output (expanded the most relevant sections, everything is folded closed by default)

image

@protolambda protolambda requested a review from ajsutton May 23, 2024 01:46
@protolambda protolambda requested a review from a team as a code owner May 23, 2024 01:46
Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

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

Can we specify the version of the fork diff docker image to use explicitly? Otherwise we risk CI having it cached or just generally having something incompatible deployed to the latest version of that docker image in the future which then breaks things.

Otherwise LGTM.

@protolambda
Copy link
Contributor Author

@ajsutton updated forkdiff repo with automatic docker builds, and tagged a v0.1.0 (0.1.0 in docker tag style), see:
https://hub.docker.com/layers/protolambda/forkdiff/0.1.0/images/sha256-4bb900ab4e097780452e4672cf1f55b967d7e5cd0e8b73807339a6868e94bd2a?context=explore

And updated forkdiff to use ignore, not ignored, to match the new local-ignore option and the old global-ignore option naming.

@protolambda protolambda enabled auto-merge (squash) May 23, 2024 11:37
@protolambda protolambda merged commit 966c435 into optimism May 23, 2024
9 of 10 checks passed
@protolambda protolambda deleted the forkdiff-improvements branch May 23, 2024 11:47
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.

None yet

2 participants