Skip to content

Removed leading spaces in yaml file#8324

Closed
pslawless wants to merge 2 commits intoelastic:masterfrom
pslawless:gh-8268/yml-spaces
Closed

Removed leading spaces in yaml file#8324
pslawless wants to merge 2 commits intoelastic:masterfrom
pslawless:gh-8268/yml-spaces

Conversation

@pslawless
Copy link

Resolves #8268

@elasticmachine
Copy link
Contributor

Can one of the admins verify this patch?


# Set the interval in milliseconds to sample system and process performance
# metrics. Minimum is 100ms. Defaults to 5000.
# ops.interval: 5000
Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome, thanks for submitting this one! Can I suggest you maintain the spaces before the non-config sections, ie the actual descriptive comments, and just remove them for the config options. Like so;

# Kibana is served by a back end server. This setting specifies the port to use.
#server.port: 5601

Take a look at https://github.com/elastic/elasticsearch/pull/20094/files, which is the Elasticsearch version of the same thing :)

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense. Will check in with that update shortly. Thanks!

Copy link
Contributor

@markwalkom markwalkom left a comment

Choose a reason for hiding this comment

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

LGTM!

@Bargs
Copy link
Contributor

Bargs commented Sep 21, 2016

LGTM! Looks like there's a conflict though, could you merge in or rebase on master?

@thomasneirynck
Copy link
Contributor

Sorry @pslawless, I missed this PR, and created a new one for this (#8438). My apologies for this... fwiw, I saw you did it in ES, and really liked that change..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants