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

Add a notice about the port to access Kibana. #9622

Merged
merged 1 commit into from
Dec 23, 2016
Merged

Add a notice about the port to access Kibana. #9622

merged 1 commit into from
Dec 23, 2016

Conversation

jgbarah
Copy link
Contributor

@jgbarah jgbarah commented Dec 23, 2016

This is an updated version of #7303. I'm sorry I had not noticed about the last comments by the reviewer. I rebased the commit, changed its insertion point, and changed the text to the suggested one.

@elasticmachine
Copy link
Contributor

Can one of the admins verify this patch?

@epixa
Copy link
Contributor

epixa commented Dec 23, 2016

jenkins, test this

@epixa epixa added the review label Dec 23, 2016
@epixa
Copy link
Contributor

epixa commented Dec 23, 2016

@jgbarah Thanks for resubmitting! I have a suggestion for new phrasing after the first sentence:

Now you can point your web browser to https://localhost:5601 and start using Kibana! When running npm start, Kibana will also log that it is listening on port 5603 due to the base path proxy, but you should still access Kibana on port 5601.

@jgbarah
Copy link
Contributor Author

jgbarah commented Dec 23, 2016

Thanks for the quick review. I just amended the commit with the sentence you suggested.

@@ -140,6 +140,8 @@ Start the development server.
npm start
```

> Now you can point your web browser to https://localhost:5601 and start using Kibana! When running `npm start`, Kibana will also log that it is listening on port 5603 due to the base path proxy, but you should still access Kibana on port 5601.
Copy link
Contributor

Choose a reason for hiding this comment

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

Another thing (sorry if I missed this in the previous review): I think this paragraph should go after the "> On Windows, ..." paragraph, and it should not include a > so it appears as normal text. So it would look like this:

screen shot 2016-12-23 at 12 51 54 pm

This way the "on windows" thing is still explaining a needs for running the previous command, whereas the new text is simply explaining what's next.

@jgbarah
Copy link
Contributor Author

jgbarah commented Dec 23, 2016

I understand, and I agree this is better. New version committed.

@epixa
Copy link
Contributor

epixa commented Dec 23, 2016

jenkins, test this

@epixa epixa self-assigned this Dec 23, 2016
@epixa
Copy link
Contributor

epixa commented Dec 23, 2016

Thanks for this!

@epixa epixa merged commit 018c3f4 into elastic:master Dec 23, 2016
elastic-jasper added a commit that referenced this pull request Dec 23, 2016
Backports PR #9622

**Commit 1:**
Add a notice about the port to access Kibana.

* Original sha: 23e2fbf
* Authored by Jesus M. Gonzalez-Barahona <jgb@gsyc.es> on 2016-05-26T22:52:52Z
elastic-jasper added a commit that referenced this pull request Dec 23, 2016
Backports PR #9622

**Commit 1:**
Add a notice about the port to access Kibana.

* Original sha: 23e2fbf
* Authored by Jesus M. Gonzalez-Barahona <jgb@gsyc.es> on 2016-05-26T22:52:52Z
epixa pushed a commit that referenced this pull request Dec 23, 2016
Backports PR #9622

**Commit 1:**
Add a notice about the port to access Kibana.

* Original sha: 23e2fbf
* Authored by Jesus M. Gonzalez-Barahona <jgb@gsyc.es> on 2016-05-26T22:52:52Z
epixa pushed a commit that referenced this pull request Dec 23, 2016
Backports PR #9622

**Commit 1:**
Add a notice about the port to access Kibana.

* Original sha: 23e2fbf
* Authored by Jesus M. Gonzalez-Barahona <jgb@gsyc.es> on 2016-05-26T22:52:52Z
@jgbarah jgbarah deleted the bootstrap branch December 24, 2016 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants