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 support for a few more Elasticsearch integration options #169

Closed
wants to merge 2 commits into from
Closed

Add support for a few more Elasticsearch integration options #169

wants to merge 2 commits into from

Conversation

chrissnell
Copy link
Contributor

Adds support for is_external, username, password, and tags.

@miketheman
Copy link
Contributor

Looks good! There's a one-space off in the tags section around the if/end statements - can you fix that?

@chrissnell
Copy link
Contributor Author

Hi @miketheman , does this new commit fix what you're talking about?

@miketheman
Copy link
Contributor

@chrissnell no, I was referring to the 3-spaces on the if/end - it should be 2 - lines 15 & 17

@chrissnell
Copy link
Contributor Author

Ahh, I see now. Better?

@chrissnell
Copy link
Contributor Author

OK, I added a space to make it a three-space to match the rest of the spacing in the template. If you want two-space everywhere, LMK and I will change everything to match.

@miketheman
Copy link
Contributor

I think mentioned that it should be 2 in my prior comment: #169 (comment)

This is how I would expect to see the tags block indented:

    <% if i.key?('tags') -%>
    tags:
      <% i['tags'].each do |t| -%>
      - <%= t %>
      <% end -%>
    <% end -%>

3-spacing is a very odd one - that why I pointed it out. 2-spacing is pretty standard Ruby and YAML.

Would you mind rebasing and squashing these commits down to a single one?

Adds support for is_external, username, password, and tags.
@chrissnell
Copy link
Contributor Author

\o/

@miketheman miketheman added this to the Next minor milestone Jan 2, 2015
@miketheman miketheman self-assigned this Jan 2, 2015
@miketheman
Copy link
Contributor

Nice! I'll merge this soon with the next round of enhancements.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 80a518c on chrissnell:master into 6f29a4c on DataDog:master.

miketheman added a commit that referenced this pull request Feb 27, 2015
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.

3 participants