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.

Sign up for GitHub

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

Jump to bottom

doc: change the order for dashboard creation note. #875

Merged
merged 2 commits into from
Sep 9, 2020
Merged

doc: change the order for dashboard creation note. #875

merged 2 commits into from
Sep 9, 2020

Conversation

Colstuwjx
Copy link
Contributor

@Colstuwjx Colstuwjx commented Sep 7, 2020 •
edited
Loading

What problem does this PR solve?

Fixed #849 .

What is changed and how does it work?

Just move the note about dashboard creation to the beginning of step3.

Tests

  • Unit test
  • E2E test
  • Manual test (add detailed scripts or steps below)
  • No code
NONE

@Colstuwjx Colstuwjx force-pushed the fix/doc-mention-set-dashboard-create branch from 433e4e2 to 4b5b930 Compare September 7, 2020 09:28
@Colstuwjx
Copy link
Contributor Author

It seems that the page didn't change the order even though I've changed the content...
See preview.

@cwen0
Copy link
Member

cwen0 commented Sep 7, 2020

It seems that the page didn't change the order even though I've changed the content...
See preview.

@g1eny0ung PTAL

@zhouqiang-cl
Copy link
Contributor

@Colstuwjx thank you for your contribution

@g1eny0ung
Copy link
Member

It seems that the page didn't change the order even though I've changed the content...
See preview.

@Colstuwjx @cwen0 Because only the latest document is modified.

If needed, synchronize changes to versioned_docs/version-0.9.0 and versioned_docs/version-0.9.1 to make all versions take effect.

@Colstuwjx
Copy link
Contributor Author

After change to Next version, it seems to be the latest version right now:

image
image

I think it's problematic, since we're usually aimed to change the latest content ( a.k.a the master branch content ), and the preview url should also default to be the latest version content, rather than the latest stable version.

@g1eny0ung
Copy link
Member

g1eny0ung commented Sep 8, 2020 •
edited
Loading

After change to Next version, it seems to be the latest version right now:

image
image

I think it's problematic, since we're usually aimed to change the latest content ( a.k.a the master branch content ), and the preview url should also default to be the latest version content, rather than the latest stable version.

Since the latest version is more likely to have some unknown bugs, from the perspective of users, we put the latest stable version in the default position. Of course, we also hope that users can use the latest version so that we can find more potential problems.

I will update the doc README later to describe how to contribute so that everyone can understand the structure of the docs.

@Colstuwjx
Copy link
Contributor Author

There are two cases, one is the website for production use, one is the preview. I mean that we could consider to set the preview url with latest version content.

@g1eny0ung
Copy link
Member

There are two cases, one is the website for production use, one is the preview. I mean that we could consider to set the preview url with latest version content.

Understood, I will consider this situation, thanks for the suggestion.~

@Colstuwjx
Copy link
Contributor Author

Just another suggestion for our doc website, I noticed that we're using netlify to serve the deploy preview, how about expose the netlify.toml configuration into our repo, like k8s website did, it could make this part more visible and maintainable.

Refer to netlify config doc for details.
Thanks.

@g1eny0ung
Copy link
Member

Just another suggestion for our doc website, I noticed that we're using netlify to serve the deploy preview, how about expose the netlify.toml configuration into our repo, like k8s website did, it could make this part more visible and maintainable.

Refer to netlify config doc for details.
Thanks.

Good suggestion. I will open an issue to track this.

@g1eny0ung g1eny0ung mentioned this pull request Sep 9, 2020
Closed
zhouqiang-cl
zhouqiang-cl approved these changes Sep 9, 2020
View reviewed changes
Copy link
Contributor

@zhouqiang-cl zhouqiang-cl left a comment

Choose a reason for hiding this comment

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

LGTM

@zhouqiang-cl
Copy link
Contributor

/merge

@ti-srebot
Copy link
Contributor

Your auto merge job has been accepted, waiting for:

  • 877

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit e233834 into chaos-mesh:master Sep 9, 2020
@Colstuwjx Colstuwjx deleted the fix/doc-mention-set-dashboard-create branch September 9, 2020 07:34
STRRL pushed a commit to STRRL/chaos-mesh that referenced this pull request Sep 10, 2020
@Colstuwjx @STRRL
doc: change the order for dashboard creation note. (chaos-mesh#875)
f8395dd 
Signed-off-by: STRRL <[email protected]>
sjwsl pushed a commit to sjwsl/chaos-mesh that referenced this pull request May 6, 2021
@Colstuwjx
doc: change the order for dashboard creation note. (chaos-mesh#875)
98ee202
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ti-srebot ti-srebot ti-srebot approved these changes

@zhouqiang-cl zhouqiang-cl zhouqiang-cl approved these changes

@g1eny0ung g1eny0ung g1eny0ung approved these changes

Assignees
No one assigned
Labels
contribution status/can-merge status/LGT2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Inconsistent from doc and chart yaml
5 participants
@Colstuwjx @cwen0 @zhouqiang-cl @g1eny0ung @ti-srebot