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

Nitpicks: is_some() refactor #1642

Merged
merged 3 commits into from
Oct 13, 2023
Merged

Nitpicks: is_some() refactor #1642

merged 3 commits into from
Oct 13, 2023

Conversation

ChihChengLiang
Copy link
Collaborator

Description

During the review of the codebase, I found some nitpicks in Options and pattern matching. Here's a quick PR to improve readability.

Type of change

Nitpick

Rationale

  • Nested conditions hurt readability, match patterns when possible
  • unwrapping Options inside a branch beats the purpose of Options

@github-actions github-actions bot added the crate-zkevm-circuits Issues related to the zkevm-circuits workspace member label Oct 4, 2023
Copy link
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.

Nice refactor!

Copy link
Member

@hero78119 hero78119 left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@ChihChengLiang ChihChengLiang added this pull request to the merge queue Oct 13, 2023
Merged via the queue into main with commit 265f561 Oct 13, 2023
13 checks passed
@ChihChengLiang ChihChengLiang deleted the is_some-refactor branch October 13, 2023 04:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
crate-zkevm-circuits Issues related to the zkevm-circuits workspace member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants