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

chore: remove &self in AccountTransaction::handle_fee #1817

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Subsegment
Copy link

@Subsegment Subsegment commented Apr 22, 2024

#1770


This change is Reviewable

crates/blockifier/src/transaction/account_transaction.rs Outdated Show resolved Hide resolved
rust-toolchain.toml Outdated Show resolved Hide resolved
@Subsegment Subsegment changed the title Remove &self in AccountTransaction::handle_fee and add rust toolchain Remove &self in AccountTransaction::handle_fee Apr 23, 2024
@tdelabro
Copy link
Contributor

@noaov1 any opinion?

noaov1
noaov1 previously approved these changes Apr 27, 2024
Copy link
Collaborator

@noaov1 noaov1 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Subsegment and @tdelabro)


crates/blockifier/src/transaction/account_transaction.rs line 306 at r3 (raw file):

    fn handle_fee<S: StateReader>(

Why did you add an additional new line?

Code quote:

    }


    fn handle_fee<S: StateReader>(

@Subsegment
Copy link
Author

Reviewed 1 of 2 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Subsegment and @tdelabro)

crates/blockifier/src/transaction/account_transaction.rs line 306 at r3 (raw file):

    fn handle_fee<S: StateReader>(

Why did you add an additional new line?

Code quote:

    }


    fn handle_fee<S: StateReader>(

It may be a misoperation during merge conflict. I will modify it

Copy link
Collaborator

@noaov1 noaov1 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @tdelabro)

@tdelabro
Copy link
Contributor

@noaov1 thanks for the reviews. Can you also trigger the workflow?

@Subsegment Subsegment changed the title Remove &self in AccountTransaction::handle_fee chore: remove &self in AccountTransaction::handle_fee May 6, 2024
@Subsegment Subsegment requested a review from noaov1 May 6, 2024 03:24
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

3 participants