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 add radius to existing app tutorial #1053

Merged
merged 3 commits into from
Feb 22, 2024

Conversation

willtsai
Copy link
Contributor

Thank you for helping make the Radius documentation better!

Please follow this checklist before submitting:

  • Read the contribution guide
  • Commands include options for Linux, MacOS, and Windows within codetabs
  • New file and folder names are globally unique
  • Page references use shortcodes instead of markdown or URL links
  • Images use HTML style and have alternative text
  • Places where multiple code/command options are given have codetabs

In addition, please fill out the following to help reviewers understand this pull request:

Description

Update the "Add Radius to an existing application" tutorial to use images in our own Radius container registry and also update the commands use to see the Radius app graph and connections.

Issue reference

Fixes: radius-project/samples#914
Related: radius-project/samples#986

Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>
@@ -258,7 +258,7 @@ You will now add Radius to the Guestbook application's Kubernetes deployment man
Now that Radius has been enabled for your application, run this command again:

```bash
rad app connections -a demo
rad app graph -a demo -g default-demo
Copy link
Contributor

Choose a reason for hiding this comment

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

Shd we change rad app connections to rad app graph in all the docs that references connection ? I don't see a CLI reference page for rad app graph in our docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

even though it's a bit beyond the scope of this PR, I agree with your observation and committed a "rebrand" of rad app connections to rad app graph -- please take a look.

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 reference pages shouldn't be manually edited. We can't update these until we complete the migration of the CLI code from connections to graph

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 its better to update this once rebranding feature is complete ?

Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>
@willtsai willtsai requested a review from Reshrahim February 22, 2024 18:39
@willtsai willtsai changed the title update add radius to existing app tutorial update add radius to existing app tutorial and rebrand rad app connections to rad app graph Feb 22, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

this page is autogenerated and shouldn't be edited directly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it, I will remove the edits from this pr. how do we go about making it auto-generate rad app graph instead of connections?

Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need to complete the implementation of radius-project/radius#6473

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it, reverted the "rebranding" changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

This page is autogenerated and shouldn't be edited directly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted.

@willtsai willtsai changed the title update add radius to existing app tutorial and rebrand rad app connections to rad app graph update add radius to existing app tutorial Feb 22, 2024
Signed-off-by: Will Tsai <28876888+willtsai@users.noreply.github.com>
@willtsai willtsai merged commit 21797a2 into radius-project:v0.30 Feb 22, 2024
9 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.

"Existing application" tutorial failed since the container tag was removed from repository.
3 participants