Skip to content

Comments

Fix/1138 followup#1185

Merged
ElliotFriend merged 8 commits intostellar:mainfrom
theahaco:fix/1138-followup
Jan 29, 2025
Merged

Fix/1138 followup#1185
ElliotFriend merged 8 commits intostellar:mainfrom
theahaco:fix/1138-followup

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!!

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