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.

Sign up for GitHub

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

Jump to bottom

Fix/migrate scarb version #2902

Open
wants to merge 53 commits into
base: master
Choose a base branch
from
Open

Fix/migrate scarb version #2902

wants to merge 53 commits into from
+528 −540

Conversation

wheval
Copy link

@wheval wheval commented Jan 31, 2025 •
edited
Loading

Closes #2651

Introduced changes

  • Fixed imports
  • Ensured all test pass
  • Upgraded scarb version to 2024_07

Checklist

  • Linked relevant issue
  • Updated relevant documentation
  • Added relevant tests
  • Performed self-review of the code
  • Added changes to CHANGELOG.md

@kkawula
Copy link
Member

kkawula commented Jan 31, 2025

Hi @wheval, there are still some errors, but you are on a good path

@wheval
Copy link
Author

wheval commented Jan 31, 2025

Yes, i am fixing those errors!

@wheval wheval requested a review from ddoktorski February 5, 2025 21:49
@wheval wheval requested a review from ddoktorski February 6, 2025 14:24
@wheval wheval requested a review from a team as a code owner February 7, 2025 10:51
@ddoktorski
Copy link
Contributor

@wheval
Many tests are currently failing. Do you plan to continue working on this and address them?

@wheval
Copy link
Author

wheval commented Feb 18, 2025

@wheval Many tests are currently failing. Do you plan to continue working on this and address them?

Yes ser, but you said i should not change the logic and some tests are meant to fail. How would i know those tests?

@wheval
Copy link
Author

wheval commented Feb 26, 2025 •
edited
Loading

One test is failing, it has to do with the MockContract, the failure data is different. Please review
@ddoktorski @kkawula

ddoktorski
ddoktorski requested changes Mar 28, 2025
View reviewed changes
Copy link
Contributor

@ddoktorski ddoktorski left a comment

Choose a reason for hiding this comment

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

@wheval

Apologize for the late response 🙏

If you want to continue working on this PR:

  • Please resolve merge conflicts
  • Address the comments
  • Run scarb fmt in all packages containing Cairo code

Let me know if you have any questions.

@wheval
Copy link
Author

wheval commented Mar 28, 2025

@ddoktorski I have made the changes!

@wheval wheval requested a review from ddoktorski March 28, 2025 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cptartur cptartur Awaiting requested review from cptartur

@kkawula kkawula Awaiting requested review from kkawula

@franciszekjob franciszekjob Awaiting requested review from franciszekjob

@ddoktorski ddoktorski Awaiting requested review from ddoktorski

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migrate all scarb packages to edition = "2024_07"
3 participants
@wheval @kkawula @ddoktorski