Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

Rm dead code except of zkevm circuits #1408

Merged
merged 10 commits into from
May 16, 2023
Merged

Rm dead code except of zkevm circuits #1408

merged 10 commits into from
May 16, 2023

Conversation

ChihChengLiang
Copy link
Collaborator

Description

We remove #![allow(dead_code)] everywhere in the codebases other than zkevm-circuits.

This PR should be merged after #1407

Issue Link

No issue yet

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Contents

  • Removed #![allow(dead_code)] everywhere other than zkevm-circuits.
  • Make test utility functions like Block::get_test_degree and Sha3CodeGen public functions so that they don't cause cargo build -p zkevm-circuits to fail. This might come with the cost of extra compilation time.
  • Refactor gen_sha3_code into Sha3CodeGen so that we don't have to import MemoryKind to use it.

@github-actions github-actions bot added crate-bus-mapping Issues related to the bus-mapping workspace member crate-eth-types Issues related to the eth-types workspace member crate-gadgets Issues related to the gadgets workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member labels May 14, 2023
@ChihChengLiang ChihChengLiang force-pushed the rm-dead-code-before-zkevm-circuits branch from 3e38fb6 to e35a2bf Compare May 16, 2023 10:37
@ChihChengLiang ChihChengLiang changed the base branch from move-stats-to-bin to main May 16, 2023 10:37
@ChihChengLiang ChihChengLiang marked this pull request as ready for review May 16, 2023 10:41
ed255
ed255 approved these changes May 16, 2023
View reviewed changes
Copy link
Member

@ed255 ed255 left a comment

Choose a reason for hiding this comment

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

LGTM! Nice clean up!

@ChihChengLiang
Copy link
Collaborator Author

ChihChengLiang commented May 16, 2023 •
edited
Loading

Thanks @ed255 for the review! I'll merge this with one review for two reasons.

  1. The PR is not too complicated and does not touch core logic.
  2. The CI is not too stable recently. Waiting for other reviews might take more rounds of CI run.

@ChihChengLiang ChihChengLiang added this pull request to the merge queue May 16, 2023
Merged via the queue into main with commit ef13927 May 16, 2023
@ChihChengLiang ChihChengLiang deleted the rm-dead-code-before-zkevm-circuits branch May 17, 2023 08:12
@ed255 ed255 mentioned this pull request Jun 2, 2023
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ed255 ed255 ed255 approved these changes

Assignees
No one assigned
Labels
crate-bus-mapping Issues related to the bus-mapping workspace member crate-eth-types Issues related to the eth-types workspace member crate-gadgets Issues related to the gadgets workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ChihChengLiang @ed255