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

chore: Cleanup proofs actions #11858

Merged
merged 5 commits into from
Sep 12, 2024
Merged

chore: Cleanup proofs actions #11858

merged 5 commits into from
Sep 12, 2024

Conversation

clabby
Copy link
Member

@clabby clabby commented Sep 11, 2024

Overview

Cleans up the proofs action tests, moving into the proofs package and supplying more helpers. Diff is a bit jarring, had to make some things in actions public for other packages' consumption.

@clabby clabby requested review from a team as code owners September 11, 2024 23:34
@clabby clabby requested review from mslipper and geoknee and removed request for a team September 11, 2024 23:34
@clabby clabby mentioned this pull request Sep 11, 2024
Merged
@clabby Graphite App
Copy link
Member Author

clabby commented Sep 11, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @clabby and the rest of your teammates on Graphite Graphite

@clabby clabby mentioned this pull request Sep 11, 2024
Merged
ajsutton
ajsutton approved these changes Sep 11, 2024
View reviewed changes
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.

Nice reduction in boilerplate. I think it would be good to separate the tests and helper code but I'm open to making that change separately - there's only so much we should do in one PR.

op-e2e/actions/proofs/simple_program_test.go Show resolved Hide resolved
op-e2e/actions/proofs/env.go Outdated Show resolved Hide resolved
@semgrep-app semgrep-app
Copy link
Contributor

semgrep-app bot commented Sep 11, 2024

Semgrep found 1 math-random-used finding:

  • op-e2e/actions/op_program_test.go

Do not use math/rand. Use crypto/rand instead.

Ignore this finding from math-random-used.

@clabby clabby force-pushed the cl/program-action-fixture-dump branch from 01501f7 to efad023 Compare September 11, 2024 23:57
@clabby clabby force-pushed the cl/program-action-fixture-dump branch from efad023 to d583a2e Compare September 12, 2024 00:32
@clabby clabby requested a review from a team as a code owner September 12, 2024 00:32
@clabby clabby requested review from refcell and removed request for a team September 12, 2024 00:32
Base automatically changed from cl/program-action-fixture-dump to develop September 12, 2024 00:50
@clabby clabby mentioned this pull request Sep 12, 2024
Merged
@clabby clabby added this pull request to the merge queue Sep 12, 2024
Merged via the queue into develop with commit 77f7268 Sep 12, 2024
61 checks passed
@clabby clabby deleted the cl/reorg-proofs-actions branch September 12, 2024 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@semgrep-app semgrep-app semgrep-app left review comments

@ajsutton ajsutton ajsutton approved these changes

@mslipper mslipper Awaiting requested review from mslipper mslipper is a code owner automatically assigned from ethereum-optimism/go-reviewers

@geoknee geoknee Awaiting requested review from geoknee geoknee is a code owner automatically assigned from ethereum-optimism/op-stack

@refcell refcell Awaiting requested review from refcell refcell is a code owner automatically assigned from ethereum-optimism/proofs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@clabby @ajsutton