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

Remove deprecated, unreachable_code and add docs to Evm circuit#1524

Merged
ChihChengLiang merged 8 commits into
mainfrom
evm-circuit-cleanup
Jul 12, 2023
Merged

Remove deprecated, unreachable_code and add docs to Evm circuit#1524
ChihChengLiang merged 8 commits into
mainfrom
evm-circuit-cleanup

Conversation

@ChihChengLiang
Copy link
Copy Markdown
Collaborator

@ChihChengLiang ChihChengLiang commented Jul 6, 2023 •
edited
Loading

Description

  • rm deprecated, unreachable_code
  • doc some parts.

Issue Link

part of #1494

Type of change

  • New feature (non-breaking change which adds functionality)

@github-actions github-actions Bot added the crate-zkevm-circuits Issues related to the zkevm-circuits workspace member label Jul 6, 2023
@ChihChengLiang ChihChengLiang linked an issue Jul 6, 2023 that may be closed by this pull request
Force documentation in EVM Circuit #1494
Closed
@ChihChengLiang ChihChengLiang changed the title Evm circuit cleanup Remove deprecated, unreachable_code and add docs to Evm circuit Jul 6, 2023
@ChihChengLiang
Copy link
Copy Markdown
Collaborator Author

ChihChengLiang commented Jul 6, 2023

I realized removing #![allow(missing_docs)] doesn't enforce documentation in constraint builders.
But it enforces documentation in public structs that were introduced when moving stats to bin.

@github-actions github-actions Bot added CI Issues related to the Continuous Integration mechanisms of the repository. crate-circuit-benchmarks Issues related to the circuit-benchmarks workspace member crate-integration-tests Issues related to the integration-tests workspace member T-bench Type: benchmark improvements labels Jul 7, 2023
@github-actions github-actions Bot removed crate-circuit-benchmarks Issues related to the circuit-benchmarks workspace member crate-integration-tests Issues related to the integration-tests workspace member T-bench Type: benchmark improvements CI Issues related to the Continuous Integration mechanisms of the repository. labels Jul 10, 2023
@ChihChengLiang ChihChengLiang marked this pull request as ready for review July 10, 2023 15:09
@ChihChengLiang ChihChengLiang marked this pull request as draft July 10, 2023 15:10
@ChihChengLiang ChihChengLiang marked this pull request as ready for review July 10, 2023 17:05
@hero78119 hero78119 self-requested a review July 11, 2023 03:42
KimiWu123
KimiWu123 approved these changes Jul 12, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@KimiWu123 KimiWu123 left a comment

Choose a reason for hiding this comment

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

LGTM!

hero78119
hero78119 approved these changes Jul 12, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@hero78119 hero78119 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM! (peeking a small typo :P)

Comment thread zkevm-circuits/src/evm_circuit.rs Outdated
@ChihChengLiang ChihChengLiang added this pull request to the merge queue Jul 12, 2023
Merged via the queue into main with commit 26bf6ba Jul 12, 2023
@ChihChengLiang ChihChengLiang deleted the evm-circuit-cleanup branch July 12, 2023 07:39
@adria0 adria0 mentioned this pull request Jul 19, 2023
Merged
github-merge-queue Bot pushed a commit that referenced this pull request Jul 21, 2023
@adria0
Fix testool manifest and add test it in github actions (#1535)
350a7a2 
### Description

Testool failed to compile after
#1524,
this is due:

- testool compilation is not checked in github actions
- some functions required are not only available without `test-circuits`
feature

### Type of change

Bug fix (non-breaking change which fixes an issue)

### Contents

- Fixes Cargo.toml
- Adds basic test of testool in github actions

---------

Co-authored-by: adria0 <adria0@nowhere>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@hero78119 hero78119 hero78119 approved these changes

@KimiWu123 KimiWu123 KimiWu123 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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.

Force documentation in EVM Circuit

3 participants

@ChihChengLiang @hero78119 @KimiWu123