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

Fix/1138 followup #1185

Merged
merged 8 commits into from
Jan 29, 2025
Merged

Fix/1138 followup #1185

merged 8 commits into from
Jan 29, 2025

Conversation

elizabethengelman
Copy link
Contributor

@elizabethengelman elizabethengelman commented Jan 16, 2025

Some additional fixes/updates that i noticed while looking into 1138

getting start

  • updated hello world and increment contracts to be the same as they are in the examples repo
  • use aliases when deploying so that our first contract bindings command works in dapp-frontend.mdx
  • add a tip about the --alias flag
  • update description of init command, and how the --name flag works

dapp-frontend.mdx

  • have the user copy the contract ids from .stellar over into the irst-soroban-app directory so we can reuse the deployments from the first secions
  • use alias for first contract bindings example
  • small update to the Counter.astro code

@stellar-jenkins
Copy link

@elizabethengelman elizabethengelman marked this pull request as ready for review January 16, 2025 21:41
@briwylde08
Copy link
Contributor

Hi @elizabethengelman! Is 1138 an issue somewhere?

@elizabethengelman
Copy link
Contributor Author

Is 1138 an issue somewhere?

@briwylde08 Good question - I was doing this work as a follow up to Jane's PR that was addressing #1138

Copy link
Contributor

@ElliotFriend ElliotFriend left a comment

Choose a reason for hiding this comment

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

Looks great! I left a couple minor thoughts/questions, but I think it's pretty close to ready as-is! Thanks @elizabethengelman!!

```

Many of the types available in typical Rust programs, such as `std::vec::Vec`, are not available, as there is no allocator and no heap memory in Soroban contracts. The `soroban-sdk` provides a variety of types like `Vec`, `Map`, `Bytes`, `BytesN`, `Symbol`, that all utilize the Soroban environment's memory and native capabilities. Primitive values like `u128`, `i128`, `u64`, `i64`, `u32`, `i32`, and `bool` can also be used. Floats and floating point math are not supported.

Contract inputs must not be references.

The `#[contract]` attribute designates the Contract struct as the type to which contract functions are associated. This implies that the struct will have contract functions implemented for it.
The `#[contract]` attribute designates the `HelloContract` struct as the type to which contract functions are associated. This implies that the struct will have contract functions implemented for it.
Copy link
Contributor

Choose a reason for hiding this comment

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

The CLI (at least v22.2.0, which I think is the current version) creates a struct called Contract.

Edit: Never mind, we're talking about the astro template, not the stellar contract init command... 🤦🏻‍♂️

Edit Edit: Oh, wait, no... This page isn't talking about the astro template, right? We might need to modify instances of HelloContract to just Contract then?

Copy link
Contributor Author

@elizabethengelman elizabethengelman Jan 29, 2025

Choose a reason for hiding this comment

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

Good catch! You're right, the current version of the CLI just has Contract.

I have a branch to update the code that stellar contract init creates to make it HelloContract so that it matches the soroban-examples repo but I forgot to open a PR!

I'll revert this back to Contract for now. And we can make this change when the cli change is also updated. I can open 2 new PRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For reference, here are the 2 new Prs: #1239 and #1185

Co-authored-by: Elliot Voris <elliot@voris.me>
@stellar-jenkins
Copy link

@elizabethengelman
Copy link
Contributor Author

@ElliotFriend just made the updates from your review - thank you! I also noticed one more small thing that needed to be fix, and pushed up a commit for that as well.

@stellar-jenkins
Copy link

1 similar comment
@stellar-jenkins
Copy link

Copy link
Contributor

@ElliotFriend ElliotFriend left a comment

Choose a reason for hiding this comment

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

looks great @elizabethengelman!! Thanks so much!!

@ElliotFriend ElliotFriend merged commit 3c2ab5a into stellar:main Jan 29, 2025
2 checks passed
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.

4 participants