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

Support multiple zabbix_alias #435

Merged
merged 1 commit into from
Oct 23, 2017
Merged

Support multiple zabbix_alias #435

merged 1 commit into from
Oct 23, 2017

Conversation

fribergr
Copy link
Contributor

Enhancement for supporting multiple Alias-directives inside zabbix_agentd.conf.

@bastelfreak
Copy link
Member

Hi @fribergr, thanks for this PR!

  • Can you please rebase against our latest master? This should fix the acceptance tests
  • Can you please add a few unit tests to verify the file content?

@bastelfreak bastelfreak added enhancement New feature or request needs-rebase labels Oct 21, 2017
@fribergr
Copy link
Contributor Author

I've added tests and rebased it against master, should be ok now.

@juniorsysadmin
Copy link
Member

I'm uncomfortable with the abuse of Ruby internals rather than strict typing (requiring the value to be an array), but won't block the change.

@bastelfreak
Copy link
Member

@juniorsysadmin good catch. Our next release already has a breaking change in it. So should this one here be changed to an array? Or accept string or array as a param in the module, but convert it to an array which gets passed to the template. We do that in other modules as well.

@juniorsysadmin
Copy link
Member

@bastelfreak I would change it to an array. Less code complexity.

@bastelfreak
Copy link
Member

@fribergr could you please change the datatype to an array?

@fribergr
Copy link
Contributor Author

@bastelfreak I just removed string support. Does it look ok now?

@bastelfreak
Copy link
Member

Can you add the datatype Optional[Array] to the zabbix_alias param in agent.pp?

@fribergr
Copy link
Contributor Author

I added the requested Optional[Array], but Travis seems broken (build stuck in queued). Anything else needed?

@bastelfreak
Copy link
Member

I restarted the job. Secret tip: You can achive the same by closing and reopening the pull request.

@bastelfreak bastelfreak merged commit 6b77f0f into voxpupuli:master Oct 23, 2017
@fribergr fribergr deleted the agent-multiple-alias-support branch October 23, 2017 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants