Prometheus libvirt exporter charm

Merge ~woutervb/charm-prometheus-libvirt-exporter:bug/1873371 into charm-prometheus-libvirt-exporter:master

  1. Git
  2. lp:~woutervb/charm-prometheus-libvirt-exporter
  3. bug/1873371
  4. Merge into master
Proposed by Wouter van Bommel
Status: Rejected
Rejected by: Xav Paice
Proposed branch: ~woutervb/charm-prometheus-libvirt-exporter:bug/1873371
Merge into: charm-prometheus-libvirt-exporter:master
Diff against target: 219 lines (+128/-3)
10 files modified
.gitignore (+4/-0)
Makefile (+19/-0)
config.yaml (+14/-0)
dev/null (+0/-1)
layer.yaml (+5/-1)
metadata.yaml (+2/-0)
reactive/libvirt_exporter.py (+31/-1)
tests/unit/test_libvirt_exporter.py (+28/-0)
tests/unit/test_requirements.txt (+4/-0)
tox.ini (+21/-0)
Reviewer Review Type Date Requested Status
Xav Paice (community) Disapprove
Alvaro Uria (community) Needs Fixing
Review via email: mp+382466@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Xav Paice (xavpaice) wrote :

LGTM, one comment but possibly for a future change when adding functional testing.

review: Approve
Revision history for this message
Alvaro Uria (aluria) wrote :

Hey Wouter,

I think this change is superseded by [1]. Dave's MP adds nrpe support and functional tests. I've asked Dave to move the functional tests into ./tests/functional and possibly remove the unit tests details because there are no unit tests. He is not on Dev rotation but if you have time, could you take on his work and create a single MP?

Thank you. I'll mark this message as "Needs fixing" and the MP as "WIP" to unlist it from the pending reviews.

1. https://code.launchpad.net/~dmzoneill/charm-prometheus-libvirt-exporter/+git/charm-prometheus-libvirt-exporter/+merge/380729

review: Needs Fixing
Revision history for this message
Xav Paice (xavpaice) wrote :

Given this change has been superseded by a different one, and it's unlikely that the original author is going to get the opportunity to re-work the change, I'll mark this one as closed now.

review: Disapprove

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

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

to all changes: