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

docs: modify to detailed links + add details to running-locally #11405

Merged
merged 6 commits into from
Jul 24, 2023

Conversation

da-head0
Copy link
Contributor

Fixes #TODO

Motivation

When I first tried to install Argo Workflows using Dev Containers in my local environment, I was confused because the link to the The Visual Studio Code Remote - Containers and Dev-container CLI in the documentation didn't go to the link to extension or Dev-Container CLI itself, even though the context indicates that this is a link for that.

Also, as someone new to Dev Containers, I didn't know that I needed to use the make clean & make start to boilerplate, so I had to infer from the #Developing Locally section in the middle of the documentation how to get Argo Workflows running in my local environment.

The changes I've made to the documentation are located earlier in the documentation and would make getting up and running with Argo Workflows faster and easier for someone like me who is new to Argo Workflows or Dev Containers.

Modifications

  • Modified the link of Dev Container extension in Visual Studio Code
  • Modified the link of Dev Container CLI
  • Added command for start the argo workflow inside Dev Container

Verification

Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

@Joibel Could you help review this?

@da-head0
Copy link
Contributor Author

da-head0 commented Jul 20, 2023

@Joibel Could you help review this?

@terrytangyuan Thanks for your help. I was confused because all the spelling errors in the checks from the original running locally.md...

Signed-off-by: Dahye Ahn <andyourturntome@gmail.com>
Signed-off-by: Dahye Ahn <andyourturntome@gmail.com>
fix typo

Signed-off-by: Dahye Ahn <andyourturntome@gmail.com>
@da-head0 da-head0 force-pushed the master branch 2 times, most recently from d0e6cb9 to 39a157d Compare July 20, 2023 15:36
docs/running-locally.md Outdated Show resolved Hide resolved
@agilgur5
Copy link
Member

agilgur5 commented Jul 23, 2023

Dev-container CLI in the documentation didn't go to the link to extension or Dev-Container CLI itself

Good change! I think I stumbled upon this myself and just forgot about it as I don't use the CLI

so I had to infer from the #Developing Locally section in the middle of the documentation how to get Argo Workflows running in my local environment.

Hmm so it might make sense to just link downward to "Developing Locally" using its anchor #developing-locally. See below comment.
(I also see that the /etc/hosts part is no longer needed for the devcontainer since #11104 automated that)

I was confused because all the spelling errors in the checks from the original running locally.md...

Not sure why these just started showing up now 🤔 Seems to be erroring on code backticks. EDIT: see below comment, I figured that out
Wonder if there was a change in mdspell recently... EDIT: nope, it hasn't had a release in 6 years

docs/running-locally.md Outdated Show resolved Hide resolved
If you want to see Argo Workflows UI in http://localhost:8080:
```
make start UI=true
````
Copy link
Member

@agilgur5 agilgur5 Jul 23, 2023

Choose a reason for hiding this comment

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

Not sure why these just started showing up now 🤔 Seems to be erroring on code backticks.

there's 4 backticks on this line instead of 3 -- that's what's causing make docs-spellcheck to fail. it's just causing syntax errors in general.

you'll then also need to change "popup" to "pop-up" to fix the spelling.

Copy link
Contributor Author

@da-head0 da-head0 Jul 24, 2023

Choose a reason for hiding this comment

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

Thanks for letting me know!!

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 changed this draft pr to open. Please review when you have the time.

Co-authored-by: Anton Gilgur <4970083+agilgur5@users.noreply.github.com>
Signed-off-by: Dahye <61692777+da-head0@users.noreply.github.com>
@da-head0 da-head0 marked this pull request as ready for review July 24, 2023 04:47
Copy link
Member

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

One small change requested, otherwise LGTM! 😃

docs/running-locally.md Show resolved Hide resolved
Signed-off-by: Dahye Ahn <andyourturntome@gmail.com>
Copy link
Member

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for working through the dev process and improving it!

@Joibel
Copy link
Member

Joibel commented Jul 24, 2023

@Joibel Could you help review this?

LGTM thanks to @agilgur5's help

@terrytangyuan terrytangyuan enabled auto-merge (squash) July 24, 2023 13:15
@terrytangyuan terrytangyuan merged commit 5f21c51 into argoproj:master Jul 24, 2023
23 checks passed
@agilgur5 agilgur5 added area/docs Incorrect, missing, or mistakes in docs area/build Build or GithubAction/CI issues labels Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build Build or GithubAction/CI issues area/docs Incorrect, missing, or mistakes in docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants