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] Improve the getting started section of the developer guide #710

Merged

Conversation

closingin
Copy link
Contributor

@closingin closingin commented Aug 6, 2021

Description

As discussed in this PR and this forum thread, running OpenSearch Dashboards using a docker image for OpenSearch is not possible straight away if you don't change the default configuration in opensearch_dashboards.yml.

I felt like it was a good idea to document it, and I used this opportunity to improve the developer guide. As the docker images have now been published, I also felt like it was a better idea to offer this way of running an OpenSearch instance, over the other two.

Feel free to tell me if you'd prefer to keep the other options!

Screenshot of the generated markdown:
Screenshot 2021-08-06 at 09-38-11 closingin OpenSearch-Dashboards at feature improve-developer-guide

Issues Resolved

N/A

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed bdd2312

@kavilla kavilla self-requested a review August 7, 2021 06:56
@@ -1,4 +1,5 @@
# OpenSearch Dashboards Developer Guide
<p align="center"><img src="https://opensearch.org/assets/brand/SVG/Logo/opensearch_logo_darkmode.svg" height="64px"/></p>
Copy link
Member

Choose a reason for hiding this comment

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

awesome! 🙌

@kavilla
Copy link
Member

kavilla commented Aug 10, 2021

This is great! Just a couple things,

Overall, there a multiple ways to get started. Your way is a lot better than our original directions but you could also extract the artifact that is published almost nightly and run the executable which is pretty easy as well (and it doesn't need to be configured). Do you think we should specify a "getting started with docker" and a "getting started with the artifact"? Regardless we definitely should get your content into the developer guide with getting started with docker for sure since your original forum issue raises a good issue.

Also, do you think you can update your commits so that the description gave a little bit more details, it would be helpful for people in the future to see the details you provided in this PR description?

@ahopp @seanneumann any opinions about the contents?

@closingin
Copy link
Contributor Author

Do you think we should specify a "getting started with docker" and a "getting started with the artifact"?

Are you talking about OSD or OS ? If it's OS, I haven't enough experience with it yet to have any opinion.

Also, do you think you can update your commits so that the description gave a little bit more details, it would be helpful for people in the future to see the details you provided in this PR description?

For sure. Do you squash commits or do you merge as is? The question behind being: do you want me to squash my commits into a single one with added description lines?

@ahopp
Copy link
Contributor

ahopp commented Aug 11, 2021

@kavilla I like the idea of adding more specific content based on the approaches available but don't want to add too much bloat if we don't feel it's needed e.g. we won't provide tutorial on how to use docker but could provide guidance on how to get started for OpenSearch specifically. The two approaches you outlined seem like a good start.

100% agree that we should get this in the developer guide though. Probably a separate issues/PR.

@ahopp
Copy link
Contributor

ahopp commented Aug 11, 2021

@closingin thanks for adding this content!

@kavilla
Copy link
Member

kavilla commented Aug 17, 2021

Are you talking about OSD or OS ? If it's OS, I haven't enough experience with it yet to have any opinion.

OS, but to Andrew's point we can merge than later.

Do you squash commits or do you merge as is? The question behind being: do you want me to squash my commits into a single one with added description lines?

Yeah we can put them into one commit with some description. Something basic is fine for example, ea9e742.

- Add more detailed explanations to setup a dev environment
- Recommend the docker version of OpenSearch as the default backend
- Improve the UI of the guide with a new header
- Remove the nvm installation section, and link to its docs instead

Signed-off-by: closingin <2735603+closingin@users.noreply.github.com>
@closingin closingin force-pushed the feature/improve-developer-guide branch from bdd2312 to 5a53dcd Compare August 17, 2021 06:30
@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 5a53dcd

@closingin
Copy link
Contributor Author

@kavilla I squashed and updated the commit messages. Tell me if it's good for you.

@closingin closingin changed the title docs: improve the developer guide [Docs] Improve the getting started section of the developer guide Aug 17, 2021
@kavilla kavilla self-requested a review August 17, 2021 17:44
Copy link
Contributor

@mihirsoni mihirsoni 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 the changes.

@mihirsoni mihirsoni merged commit 7df5e2a into opensearch-project:main Aug 18, 2021
kavilla pushed a commit to kavilla/OpenSearch-Dashboards-1 that referenced this pull request Aug 18, 2021
opensearch-project#710)

- Add more detailed explanations to setup a dev environment
- Recommend the docker version of OpenSearch as the default backend
- Improve the UI of the guide with a new header
- Remove the nvm installation section, and link to its docs instead

Backport PR:
opensearch-project#710

Signed-off-by: closingin <2735603+closingin@users.noreply.github.com>
kavilla added a commit that referenced this pull request Aug 20, 2021
#710) (#729)

- Add more detailed explanations to setup a dev environment
- Recommend the docker version of OpenSearch as the default backend
- Improve the UI of the guide with a new header
- Remove the nvm installation section, and link to its docs instead

Backport PR:
#710

Signed-off-by: closingin <2735603+closingin@users.noreply.github.com>

Co-authored-by: Rémi Weislinger <2735603+closingin@users.noreply.github.com>
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
# [24.1.0](elastic/elastic-charts@v24.0.0...v24.1.0) (2020-11-24)

### Bug Fixes

* **area_charts:** correctly represent baseline with negative data points ([opensearch-project#896](elastic/elastic-charts#896)) ([b622fda](elastic/elastic-charts@b622fda))
* **legend:** legend sizes with ordinal data ([opensearch-project#867](elastic/elastic-charts#867)) ([74bcbad](elastic/elastic-charts@74bcbad)), closes [opensearch-project#811](elastic/elastic-charts#811)
* render orphan data points on lines and areas ([opensearch-project#900](elastic/elastic-charts#900)) ([3e2c739](elastic/elastic-charts@3e2c739)), closes [opensearch-project#783](elastic/elastic-charts#783)
* specs swaps correctly reflected in state ([opensearch-project#901](elastic/elastic-charts#901)) ([a94347f](elastic/elastic-charts@a94347f))

### Features

* **legend:** allow legend text to be copyable ([opensearch-project#877](elastic/elastic-charts#877)) ([21a96d3](elastic/elastic-charts@21a96d3)), closes [opensearch-project#710](elastic/elastic-charts#710)
* allow clearing series colors from memory ([opensearch-project#899](elastic/elastic-charts#899)) ([e97f4ab](elastic/elastic-charts@e97f4ab))
* merge series domain with the domain of another group ([opensearch-project#912](elastic/elastic-charts#912)) ([716ad5a](elastic/elastic-charts@716ad5a))
* small multiples for XY charts (alpha) ([opensearch-project#793](elastic/elastic-charts#793)) ([3b88e1c](elastic/elastic-charts@3b88e1c)), closes [opensearch-project#500](elastic/elastic-charts#500) [opensearch-project#500](elastic/elastic-charts#500)
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.

5 participants