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

Update Orchestration Walkthrough #1151

Merged
merged 10 commits into from
Jul 11, 2024
Merged

Update Orchestration Walkthrough #1151

merged 10 commits into from
Jul 11, 2024

Conversation

amessbee
Copy link
Contributor

@amessbee amessbee commented Jul 10, 2024

There are similar changes for two of the walkthroughs in orchestration section: Cross Chain Swap and Cross Chain Unbond. The major change is use of the new withOrchestration API.

Refs:

Copy link

cloudflare-workers-and-pages bot commented Jul 10, 2024

Deploying documentation with  Cloudflare Pages  Cloudflare Pages

Latest commit: 78bc96f
Status: ✅  Deploy successful!
Preview URL: https://9fd85673.documentation-7tp.pages.dev
Branch Preview URL: https://ms-update-orch-walkthrough.documentation-7tp.pages.dev

View logs

Copy link

github-actions bot commented Jul 10, 2024

Cloudflare deployment logs are available here

@amessbee amessbee marked this pull request as ready for review July 10, 2024 10:08
@amessbee amessbee changed the title Update Orchestration Walkthrough (WIP) Update Orchestration Walkthrough Jul 10, 2024
Comment on lines -180 to -181
## Start Function
Now we define the main entrypoint of the contract, the `start` function, with the usual arguments, `zcf`, `privateAge`, and `baggage`:
Copy link
Member

Choose a reason for hiding this comment

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

I think we should keep explaining that the module exports a start function.

It's merely a convention/convenience that we define a more abstract contract and pass it to withOrchestration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added/updated the description to fix this. I am just documenting the code here. Not explaining contract function (or just explaining start function) may confuse the reader IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@turadg Will wait for your comments before merge.

Copy link
Member

Choose a reason for hiding this comment

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

i see. lgtm

Copy link
Contributor

@toliaqat toliaqat left a comment

Choose a reason for hiding this comment

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

I left a few comments. Please fix them before merging. Looks good to me.

## Start Function
Now we define the main entrypoint of the contract, the `start` function, with the usual arguments, `zcf`, `privateAge`, and `baggage`:
## `contract` Function
The `contract` function when wrapped inside `withOrchestration` defines the entry point of the contract., with the arguments, `zcf`, `privateAge`, `zone`, and `tools` for orchestration:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The `contract` function when wrapped inside `withOrchestration` defines the entry point of the contract., with the arguments, `zcf`, `privateAge`, `zone`, and `tools` for orchestration:
The `contract` function when wrapped inside `withOrchestration` defines the entry point of the contract, with the arguments, `zcf`, `privateAge`, `zone`, and `tools` for orchestration:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@@ -63,7 +63,6 @@ Retrieves the omniflixhub chain object using the orchestrator.
Creates an account on the omniflixhub chain.

## Placeholder for Delegation and Undelegation
TODO
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should delete these headings completely that are not explained yet, and add them when we know what to add for them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

```

Defines the `start` function of the contract as a wrapped veriosn of [`contract` function above](#contract-function) with `withOrchestration`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we rewrite this sentence, because it seems weird for some reason with as a wrapped....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@amessbee amessbee merged commit d766405 into main Jul 11, 2024
5 checks passed
@amessbee amessbee deleted the ms/update-orch-walkthrough branch July 11, 2024 14:45
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.

3 participants