Juju Charms Collection
rsyslog package

Merge lp://staging/~niedbalski/charms/precise/rsyslog/lp-1310793 into lp://staging/charms/rsyslog

  1. Precise Pangolin (12.04)
  2. lp-1310793
  3. Merge into trunk
Proposed by Jorge Niedbalski
Status: Merged
Merged at revision: 10
Proposed branch: lp://staging/~niedbalski/charms/precise/rsyslog/lp-1310793
Merge into: lp://staging/charms/rsyslog
Diff against target: 1813 lines (+1595/-70)
24 files modified
Makefile (+22/-0)
README (+0/-1)
charm-helpers.yaml (+5/-0)
config.yaml (+4/-0)
files/60-aggregator.conf (+0/-7)
hooks/charmhelpers/contrib/templating/contexts.py (+104/-0)
hooks/charmhelpers/core/hookenv.py (+401/-0)
hooks/charmhelpers/core/host.py (+297/-0)
hooks/charmhelpers/fetch/__init__.py (+308/-0)
hooks/charmhelpers/fetch/archiveurl.py (+63/-0)
hooks/charmhelpers/fetch/bzrurl.py (+49/-0)
hooks/config-changed (+0/-39)
hooks/hooks.py (+125/-0)
hooks/install (+0/-8)
hooks/start (+0/-8)
hooks/stop (+0/-4)
hooks/upgrade-charm (+0/-2)
revision (+1/-1)
setup.cfg (+5/-0)
templates/60-aggregator.conf (+7/-0)
templates/nova-logging.conf (+12/-0)
templates/rsyslog.conf (+37/-0)
test_requirements.txt (+6/-0)
unit_tests/test_hooks.py (+149/-0)
To merge this branch: bzr merge lp://staging/~niedbalski/charms/precise/rsyslog/lp-1310793
Reviewer Review Type Date Requested Status
Marco Ceppi (community) Approve
Review via email: mp+216773@code.staging.launchpad.net

Description of the change

Proposed for fix lp:1310793

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

Hey Jorge,

Thanks for this submission, it looks great! Happy to see all those unit tests! I had to make a small change to the test_hooks.py file to get `make test` to complete successfully, so that should probably be updated on your branch:

--- unit_tests/test_hooks.py 2014-04-30 20:29:29.665068579 -0400
+++ unit_tests/test_hooks.py.new 2014-04-30 20:28:02.954158683 -0400
@@ -13,7 +13,7 @@
 except ImportError as ex:
     raise ImportError("Please install unittest and mock modules")

-from hooks import hooks
+import hooks

 TO_PATCH = [
     "apt_install",

Aside from that, this looks awesome and gets a big +1 from me. Someone from the charmers team will come through soon to do a followup review and promulgate these changes into the charm store.

If you have any questions/comments/concerns about the review contact us in #juju on irc.freenode.net or email the mailing list <email address hidden>

Thanks again for this contribution!

Revision history for this message
Marco Ceppi (marcoceppi) wrote :

Hi Jorge,

In addition to the one line fix that Tim mentioned, make lint is also failing with a few flake8 errors:

$ make lint
hooks/hooks.py:5:1: F401 're' imported but unused
hooks/hooks.py:54:9: F401 'jinja2' imported but unused
make: *** [lint] Error 1

Could you address those as well prior to merging, otherwise another +1 from me, amazing work!

lp://staging/~niedbalski/charms/precise/rsyslog/lp-1310793 updated
12. By Jorge Niedbalski

[review] modified according to Tim Van Steenburgh , and Marco Ceppi observations

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

Hey guys,

Fixed.

[flake8 --exclude=charmhelpers --count] == 0
[make test] == 7 passed, 0 failed

Thanks!

lp://staging/~niedbalski/charms/precise/rsyslog/lp-1310793 updated
13. By Jorge Niedbalski

[fix] Modload imtcp not exposed yet

Revision history for this message
Marco Ceppi (marcoceppi) wrote :

LGTM, THANKS!

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