Merge lp:~bloodearnest/charms/precise/squid-reverseproxy/trunk into lp:charms/squid-reverseproxy

Proposed by Simon Davy on 2014-09-22
Status: Merged
Merged at revision: 49
Proposed branch: lp:~bloodearnest/charms/precise/squid-reverseproxy/trunk
Merge into: lp:charms/squid-reverseproxy
Diff against target: 12 lines (+1/-1)
1 file modified
hooks/install (+1/-1)
To merge this branch: bzr merge lp:~bloodearnest/charms/precise/squid-reverseproxy/trunk
Reviewer Review Type Date Requested Status
Marco Ceppi 2014-09-22 Approve on 2015-01-07
Review Queue (community) automated testing Needs Fixing on 2014-11-21
Adam Israel (community) Needs Fixing on 2014-10-29
Review via email: mp+235429@code.launchpad.net

Commit message

Ensure bash install hook doesn't fail spuriously.

Description of the change

Trivial fix, but urgent, as the install hook is broken currently, unless you happen to use the exec.d/ and basenode approach. Installing from the charmstore will always fail AFAICT, as exec.d will never exist.

To post a comment you must log in.
Review Queue (review-queue) wrote :

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-1104-results

review: Needs Fixing (automated testing)
Review Queue (review-queue) wrote :

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-1201-results

review: Needs Fixing (automated testing)
Adam Israel (aisrael) wrote :

Hi Simon,

Thanks for your work on this. I had the chance to review the charm today, and came across two issues that are preventing the charm from passing the automated tests. I realize that neither of these issues are caused by your patch, but are causing a block.

First, a small charm proof error.

$ charm proof
E: Maintainer field must not be a list

The maintainer field in metadata.yaml needs to be changed to maintainers in order to accept a list.

Second, a handful of lint errors.

Checking for Python syntax...
/vagrant/precise/squid-reverseproxy/hooks/hooks.py:99:12: E713 test for membership should be 'not in'
/vagrant/precise/squid-reverseproxy/hooks/hooks.py:149:1: E265 block comment should start with '# '
/vagrant/precise/squid-reverseproxy/hooks/hooks.py:154:1: E265 block comment should start with '# '
/vagrant/precise/squid-reverseproxy/hooks/hooks.py:161:1: E265 block comment should start with '# '
/vagrant/precise/squid-reverseproxy/hooks/hooks.py:166:1: E265 block comment should start with '# '
/vagrant/precise/squid-reverseproxy/hooks/hooks.py:174:1: E265 block comment should start with '# '
/vagrant/precise/squid-reverseproxy/hooks/hooks.py:178:1: E265 block comment should start with '# '
/vagrant/precise/squid-reverseproxy/hooks/hooks.py:187:1: E265 block comment should start with '# '
/vagrant/precise/squid-reverseproxy/hooks/hooks.py:191:1: E265 block comment should start with '# '
/vagrant/precise/squid-reverseproxy/hooks/hooks.py:203:1: E265 block comment should start with '# '
/vagrant/precise/squid-reverseproxy/hooks/hooks.py:207:1: E265 block comment should start with '# '
/vagrant/precise/squid-reverseproxy/hooks/hooks.py:225:1: E265 block comment should start with '# '
/vagrant/precise/squid-reverseproxy/hooks/hooks.py:227:1: E265 block comment should start with '# '
/vagrant/precise/squid-reverseproxy/hooks/hooks.py:358:1: E265 block comment should start with '# '
/vagrant/precise/squid-reverseproxy/hooks/hooks.py:361:1: E265 block comment should start with '# '
/vagrant/precise/squid-reverseproxy/hooks/hooks.py:418:80: E501 line too long (80 > 79 characters)
/vagrant/precise/squid-reverseproxy/hooks/tests/test_nrpe_hooks.py:50:12: E713 test for membership should be 'not in'
make: *** [lint] Error 1

Thanks again for your work on this!

review: Needs Fixing
Review Queue (review-queue) wrote :

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-10372-results

review: Needs Fixing (automated testing)
Marco Ceppi (marcoceppi) wrote :

LGTM, I've touched up the other linting errors as they are outside this merge.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'hooks/install'
2--- hooks/install 2013-10-10 22:50:35 +0000
3+++ hooks/install 2014-09-22 08:45:59 +0000
4@@ -3,7 +3,7 @@
5 set -eu
6
7 juju-log 'Invoking charm-pre-install hooks'
8-[ -d exec.d ] && ( for f in exec.d/*/charm-pre-install; do [ -x $f ] && /bin/sh -c "$f"; done )
9+[ -d exec.d ] && ( for f in exec.d/*/charm-pre-install; do [ -x $f ] && /bin/sh -c "$f"; done ) || true
10
11 juju-log 'Invoking python-based install hook'
12 python hooks/hooks.py install

Subscribers

People subscribed via source and target branches

to all changes: