Merge lp://qastaging/~jacekn/charm-helpers/nrpe-hostname into lp://qastaging/charm-helpers

Proposed by Jacek Nykis
Status: Merged
Merged at revision: 245
Proposed branch: lp://qastaging/~jacekn/charm-helpers/nrpe-hostname
Merge into: lp://qastaging/charm-helpers
Diff against target: 42 lines (+15/-2)
2 files modified
charmhelpers/contrib/charmsupport/nrpe.py (+5/-2)
tests/contrib/charmsupport/test_nrpe.py (+10/-0)
To merge this branch: bzr merge lp://qastaging/~jacekn/charm-helpers/nrpe-hostname
Reviewer Review Type Date Requested Status
Jorge Niedbalski (community) Approve
Jacek Nykis (community) Needs Resubmitting
Tim Van Steenburgh Approve
Review via email: mp+218072@code.qastaging.launchpad.net

Description of the change

Add optional hostname parameter to NRPE class. This is useful in MAAS environments where machines can have real hostnames in DNS.

To post a comment you must log in.
Revision history for this message
Tim Van Steenburgh (tvansteenburgh) wrote :

Hi Jacek, thanks for this improvement to charmhelpers!

+1 LGTM, tests all pass.

A Charmer will be along to merge these changes soon.

review: Approve
Revision history for this message
Jorge Niedbalski (niedbalski) wrote :

Hi Jacek,

Thanks for working on charmhelpers.

I added a simple test case for covering your change, please merge https://code.launchpad.net/~niedbalski/charm-helpers/nrpe-hostname/+merge/220842 into your code.

Please note that is a good practice to include a test case for cover your change.

review: Needs Fixing
146. By Jacek Nykis

Merged test from niedbalski

Revision history for this message
Jacek Nykis (jacekn) wrote :

Merged test code as requested, thank you Jorge.

review: Needs Resubmitting
Revision history for this message
Jorge Niedbalski (niedbalski) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches