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

Various improvements of the Scarb package #26

Closed

Conversation

TAdev0
Copy link
Contributor

@TAdev0 TAdev0 commented Apr 11, 2024

@b-j-roberts I improved the cairo codebase overall :

  • added the edition 2023_11 (which is now the default when creating a scarb package), which actives visibility feature and highlighted many visibility issues. Its nice to do it now as in a few release it will be mandatory to make every item that require it pub
  • improved in the way values are returned in the codebase, as a single expression without the return keyword
  • replaced the QuestClaimed event and the IPixelQuest interface at the right place for clarity

onchain/src/quests/interfaces.cairo Outdated Show resolved Hide resolved
@@ -1,2 +1,2 @@
scarb 2.6.3
starknet-foundry 0.20.0
starknet-foundry 0.21.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you update the Dockerfiles to reflect this change, like here

RUN /bin/bash /root/.local/bin/snfoundryup --version 0.20.0

Either manually change them to 0.21.0 or set up a way for the Dockerfile to properly import the changed value if you'd find that interesting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@b-j-roberts will change manually for now if this is ok for you

@TAdev0 TAdev0 closed this Apr 11, 2024
@TAdev0 TAdev0 deleted the various_improvements branch April 11, 2024 20:09
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.

2 participants