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

added deprecation to impl #7257

Merged
merged 1 commit into from
Feb 12, 2025
Merged

added deprecation to impl #7257

merged 1 commit into from
Feb 12, 2025

Conversation

dean-starkware
Copy link
Collaborator

No description provided.

@dean-starkware dean-starkware marked this pull request as ready for review February 11, 2025 14:02
@reviewable-StarkWare
Copy link

This change is Reviewable

@dean-starkware dean-starkware force-pushed the dean/deprecated_trait_items branch from 990151a to 5a00d55 Compare February 11, 2025 14:28
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 2 of 12 files at r1, all commit messages.
Reviewable status: 2 of 12 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware)


-- commits line 5 at r1:
fix rebase.

Copy link
Collaborator Author

@dean-starkware dean-starkware 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: 2 of 12 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @orizi)


-- commits line 5 at r1:

Previously, orizi wrote…

fix rebase.

what's the problem?
it's on top of the trait's PR.

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: 2 of 12 files reviewed, 1 unresolved discussion (waiting on @dean-starkware and @gilbens-starkware)


-- commits line 5 at r1:

Previously, dean-starkware wrote…

what's the problem?
it's on top of the trait's PR.

so i should see only the changes from it. after merge rebase. or before merge - change target.
(but since just merged - just rebase)

Copy link
Collaborator Author

@dean-starkware dean-starkware 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: 2 of 12 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @orizi)


-- commits line 5 at r1:

Previously, orizi wrote…

so i should see only the changes from it. after merge rebase. or before merge - change target.
(but since just merged - just rebase)

It is the target though.
Everything seems right.
Anyway- I'll rebase with main once it's merged and change the target to main.

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: 2 of 12 files reviewed, 1 unresolved discussion (waiting on @dean-starkware and @gilbens-starkware)


-- commits line 5 at r1:

Previously, dean-starkware wrote…

It is the target though.
Everything seems right.
Anyway- I'll rebase with main once it's merged and change the target to main.

still awaiting on rebase - please only respond after.

@dean-starkware dean-starkware changed the base branch from dean/deprecated_trait_items to main February 11, 2025 17:19
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:

Reviewed 1 of 12 files at r1, 11 of 11 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @gilbens-starkware)

@dean-starkware dean-starkware added this pull request to the merge queue Feb 12, 2025
Merged via the queue into main with commit 5326512 Feb 12, 2025
48 checks passed
@orizi orizi deleted the dean/deprecated_impl branch February 12, 2025 10:08
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