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

Add bool::then_some function #4529

Merged
merged 5 commits into from
Jan 11, 2024
Merged

Conversation

0xLucqs
Copy link
Contributor

@0xLucqs 0xLucqs commented Dec 8, 2023

Converts a bool in an option


This change is Reviewable

@0xLucqs 0xLucqs force-pushed the lucas/bool/then-some branch 2 times, most recently from 5428ed7 to 9f1ab70 Compare December 8, 2023 10:00
@0xLucqs 0xLucqs requested a review from orizi December 8, 2023 10:08
@0xLucqs 0xLucqs force-pushed the lucas/bool/then-some branch 8 times, most recently from 013dc21 to 4759ea4 Compare December 11, 2023 11:49
Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 12 files reviewed, 1 unresolved discussion (waiting on @LucasLvy)


corelib/src/lib.cairo line 18 at r6 (raw file):

}

impl BoolSerde of Serde<bool> {

Don't move now any of the existing impls, just create the new one in the new file.

@0xLucqs 0xLucqs force-pushed the lucas/bool/then-some branch from 4759ea4 to bf91321 Compare December 11, 2023 16:13
@0xLucqs
Copy link
Contributor Author

0xLucqs commented Dec 11, 2023

should be good now

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 12 files reviewed, 3 unresolved discussions (waiting on @LucasLvy)


corelib/src/boolean.cairo line 2 at r8 (raw file):

#[generate_trait]
pub impl ThenSome<T, +Drop<T>> of ThenSomeTrait<T> {

no reason the next added functions won't be added here.

Suggestion:

pub impl BoolImpl<T, +Drop<T>> of BoolTrait<T> {

corelib/src/lib.cairo line 101 at r8 (raw file):

}
pub mod boolean;
use boolean::ThenSome;

remove.

Code quote:

use boolean::ThenSome;

@0xLucqs 0xLucqs force-pushed the lucas/bool/then-some branch from 6167937 to 792ab33 Compare December 12, 2023 09:08
@0xLucqs 0xLucqs requested a review from orizi December 12, 2023 09:09
Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

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

Reviewed 10 of 12 files at r7, 2 of 2 files at r9, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @LucasLvy)

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @LucasLvy)

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

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

Reviewed 10 of 11 files at r10, all commit messages.
Reviewable status: 11 of 12 files reviewed, all discussions resolved (waiting on @ilyalesokhin-starkware)

Copy link
Contributor

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 2 files at r9, 11 of 11 files at r10.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @LucasLvy)

@orizi orizi added this pull request to the merge queue Jan 11, 2024
Merged via the queue into starkware-libs:main with commit 75f779e Jan 11, 2024
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.

3 participants