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

Cluster usage doc tests fix #27616

Merged
merged 1 commit into from
Dec 1, 2017
Merged

Cluster usage doc tests fix #27616

merged 1 commit into from
Dec 1, 2017

Conversation

clintongormley
Copy link
Contributor

@clintongormley clintongormley commented Dec 1, 2017 •
edited
Loading

#27611 broke the docs tests because $node_name in the URL doesn't
seem to be replaced.

Changing this to a * to match all nodes seems to fix the test

@clintongormley
elastic#27611 broke the docs tests because $node_name in the URL doesn't
303c77d 
seem to be replaced.

Changing this to a * to match all nodes seems to fix the test
@clintongormley clintongormley added the >test Issues or PRs that are addressing/adding tests label Dec 1, 2017
@clintongormley clintongormley added the >docs General docs changes label Dec 1, 2017
cbuescher
cbuescher approved these changes Dec 1, 2017
View reviewed changes
Copy link
Member

@cbuescher cbuescher left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, my bad. I'm curious to understand why changing the order of the requests in the example affects how the replacement works, but if this fixed the failing test we should merge this regardless

@clintongormley
Copy link
Contributor Author

@cbuescher by having the order switched, the result which was tested was for a node called $node_name literally, which doesn't exist.

The easier fix would be to revert the previous commit and to fix the text in the following para which refers to the order of the commands, but i think showing the simpler command first makes more sense.

@clintongormley clintongormley merged commit bf9e208 into elastic:6.1 Dec 1, 2017
@clintongormley clintongormley deleted the cluster-usage-doc-tests branch December 1, 2017 15:54
cbuescher pushed a commit that referenced this pull request Dec 1, 2017
@clintongormley @cbuescher
Fix cluster usage docs test
e1aa6e2 
#27611 broke the docs tests because $node_name in the URL doesn't (#27616)seem to be replaced.

Changing this to a * to match all nodes seems to fix the test
cbuescher pushed a commit that referenced this pull request Dec 1, 2017
@clintongormley @cbuescher
Fix cluster usage docs test
4a5a97c 
#27611 broke the docs tests because $node_name in the URL doesn't (#27616)seem to be replaced.

Changing this to a * to match all nodes seems to fix the test
cbuescher pushed a commit that referenced this pull request Dec 1, 2017
@clintongormley @cbuescher
Fix cluster usage docs test
d951bbf 
#27611 broke the docs tests because $node_name in the URL doesn't (#27616)seem to be replaced.

Changing this to a * to match all nodes seems to fix the test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cbuescher cbuescher cbuescher approved these changes

@colings86 colings86 Awaiting requested review from colings86

@nik9000 nik9000 Awaiting requested review from nik9000

Assignees
No one assigned
Labels
>docs General docs changes >test Issues or PRs that are addressing/adding tests v6.0.2 v6.1.0 v6.2.0 v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@clintongormley @cbuescher @colings86