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

Quickstarts diagrams update #4430

Merged
merged 6 commits into from
Nov 25, 2024
Merged

Quickstarts diagrams update #4430

merged 6 commits into from
Nov 25, 2024

Conversation

hhunter-ms
Copy link
Collaborator

Description

Update diagrams to match style in the new Dapr diagrams deck

Issue reference

PR will close: #4219

Signed-off-by: Hannah Hunter <[email protected]>
@hhunter-ms hhunter-ms requested review from a team as code owners November 14, 2024 15:43
@hhunter-ms hhunter-ms self-assigned this Nov 14, 2024
Copy link

Stale PR, paging all reviewers

@github-actions github-actions bot added the stale label Nov 20, 2024
Copy link
Member

@msfussell msfussell left a comment

Choose a reason for hiding this comment

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

Couple of changes

  • SmartDetectorActor should be SmokeDetectorActor - See https://docs.dapr.io/getting-started/quickstarts/actors-quickstart/ in the code
  • For Service Invocation it should say Name Resolution instead of DNS. DNS is one example in Kubernetes, but Name Resolution is general component. For example when used locally (no kubernetes) this is not DNS.

The rest look good

@hhunter-ms hhunter-ms removed the stale label Nov 20, 2024
Copy link
Member

@msfussell msfussell left a comment

Choose a reason for hiding this comment

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

Couple more suggestions to put labels on arrows and naming

  • Cron and Postgres bindings have lost the "input" and "output" labels
  • On configuration add "Get or Save Key/Value"
  • On Secret API 1) Between App and Dapr add GetSecret for the call 2) This was called OrderProcessor and now has been called MyApp. I would revert the name to Order Processor.

Copy link
Member

@msfussell msfussell left a comment

Choose a reason for hiding this comment

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

LGTM

@hhunter-ms hhunter-ms closed this Nov 25, 2024
@hhunter-ms hhunter-ms reopened this Nov 25, 2024
@hhunter-ms hhunter-ms merged commit b7b010a into dapr:v1.14 Nov 25, 2024
13 of 15 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.

Recreate new diagrams based on the new design in the Dapr slidedeck
2 participants