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

refactor: replace external(v0) by abi(embed_v0) #126

Merged
merged 3 commits into from
Nov 26, 2023

Conversation

enitrat
Copy link
Collaborator

@enitrat enitrat commented Nov 25, 2023

Issue(s): close #125

Description

  • see title
  • fixed wrong casing conventions in cheat sheet contracts (by the way, the relevancy of this section is not super obvious to me, but I guess the more the better)
  • removed implicit interfaces (no longer possible - embedded interfaces can only be implementations of a starknet::interface trait)

Checklist

  • CI Verifier: Run cairo_programs_verifier successfully
  • Contract Tests: Added tests to cover the changes
  • Remix Link: Added a link to open the contract in Remix
  • Deployed Contract (Optional): If possible, include a Voyager link to a deployed contract on Goerli

@enitrat enitrat mentioned this pull request Nov 25, 2023
4 tasks
Copy link
Contributor

@julio4 julio4 left a comment

Choose a reason for hiding this comment

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

LGTM, I just have some concerns about implicit/explicit interfaces. Thank you for taking the time to correct these issues!

About the Cheatsheet part: The idea was to have ultra concise example about common patterns (such as loop, match, type casting, ...) that new Cairo developers could refers to quickly remember the correct syntax.

src/ch00/interacting/interfaces-traits.md Outdated Show resolved Hide resolved
@enitrat
Copy link
Collaborator Author

enitrat commented Nov 26, 2023

About the Cheatsheet part: The idea was to have ultra concise example about common patterns (such as loop, match, type casting, ...) that new Cairo developers could refers to quickly remember the correct syntax.

As Cairo is not just a SC language it makes no sense to me why these would be presented inside contracts 🤔 It just introduces un-necessary boilerplate over what's actually important

@julio4
Copy link
Contributor

julio4 commented Nov 26, 2023

About the Cheatsheet part: The idea was to have ultra concise example about common patterns (such as loop, match, type casting, ...) that new Cairo developers could refers to quickly remember the correct syntax.

As Cairo is not just a SC language it makes no sense to me why these would be presented inside contracts 🤔 It just introduces un-necessary boilerplate over what's actually important

I understand your concerns and mostly agree, it's a bit out of scope for contracts, but still in the philosophy of by example. I received some feedback that these quick references are helpful for beginners. Maybe we should move it to something similar in the Cairo Book instead?

We can continue the discussion in #102 if you want.

@julio4
Copy link
Contributor

julio4 commented Nov 26, 2023

LGTM! Thank you!

@julio4 julio4 merged commit 35ec815 into main Nov 26, 2023
1 check passed
@julio4 julio4 deleted the refactor/deprecate-external branch November 26, 2023 09:34
@enitrat
Copy link
Collaborator Author

enitrat commented Nov 26, 2023

I think it could stay there but just not wrapped inside a contract 🤔

@julio4
Copy link
Contributor

julio4 commented Nov 27, 2023

I think it could stay there but just not wrapped inside a contract 🤔

Hum I agree with you on that. It will be addressed soon.

@julio4 julio4 mentioned this pull request Nov 27, 2023
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.

#[external(v0)] soon deprecated
2 participants