Conversation
[activemq_xml] fixing tests + flakes
albertvaka
left a comment
albertvaka
left a comment
There was a problem hiding this comment.
Looks good but docs need to be updated.
| let(:facts) {{ | ||
| operatingsystem: 'Ubuntu', | ||
| }} | ||
| if enabled |
There was a problem hiding this comment.
if is_agent_5 would be way more readable, since it's what really means. Why are we calling this enabled?
There was a problem hiding this comment.
I see it's written like this in every integration rspec, so it's probably better to keep it consistent unless we change it everywhere.
| <% if !instance['supress_errors'].nil? -%> | ||
| supress_errors: <%= instance['supress_errors'] %> | ||
| <%- end -%> | ||
| <%- if !instance['detailed_queues'].nil? and unless instance['detailed_queues'].empty? -%> |
There was a problem hiding this comment.
Mixing unless and if !. Would be more idiomatic to always use unless.
There was a problem hiding this comment.
You're right this isn't very idiomatic - and it's just rooted at my subpar ruby. I just always struggle with the readability of compound logical statements with unless because you never know if the other side of the or/and statement is also getting negated, and feel it ends up being less readable. I'll change it regardless.
| # The username for the datadog user | ||
| # $ssl | ||
| # Boolean to enable SSL | ||
| # $use_psycopg2 |
There was a problem hiding this comment.
docs copy-pasted from postgres integration :) and not updated.
albertvaka
left a comment
albertvaka
left a comment
There was a problem hiding this comment.
Added one comment but LGTM.
| # | ||
| # Parameters: | ||
| # $url | ||
| # URL to gather the activemq_xml starts from |
There was a problem hiding this comment.
Maybe mention this should point to ActiveMQ web admin's URL, and not to the service itself.
* [activemq_xml] adding integration [activemq_xml] fixing tests + flakes * [activemq_xml] fixing documentation * [activemq_xml][spec] improve readability * [spec] improve agent5 test switch readability * [activemq_xml] better docs
2 participants
No description provided.