Merge lp:~nextrevision/charms/precise/openvpn/trunk into lp:charms/openvpn

Proposed by John Patterson
Status: Merged
Merged at revision: 6
Proposed branch: lp:~nextrevision/charms/precise/openvpn/trunk
Merge into: lp:charms/openvpn
Diff against target: 36 lines (+3/-3)
3 files modified
config.yaml (+2/-0)
metadata.yaml (+1/-2)
revision (+0/-1)
To merge this branch: bzr merge lp:~nextrevision/charms/precise/openvpn/trunk
Reviewer Review Type Date Requested Status
Marco Ceppi (community) Approve
Matt Bruzek (community) +1 Approve
Review via email: mp+202980@code.launchpad.net

Description of the change

Correctly categorized charm as cache-proxy (a better fit IMHO compared to the previous category 'application', which was also misspelled) and updated config.yaml with defaults on all variables. Also removed the revision file, as it is no longer needed. This charm still does not provide anything with regards to interfaces, and that is still by design until someone comes up with a cool use in that case.

To post a comment you must log in.
Revision history for this message
Matt Bruzek (mbruzek) wrote :

# Proof

There is only one outstanding issue when running charm proof:

$ juju charm proof openvpn
I: all charms should provide at least one thing

# Review

+1 review. The changes made in this proposal look good to me.

# Knitpick

None.

review: Approve (+1)
Revision history for this message
Matt Bruzek (mbruzek) wrote :

Hello John,

Thanks for the improvements to the openvpn charm. My review has not found any problems with this merge set but I am not yet a charmer so someone else will be along to review this soon.

The categorization looks correct now and the revision file is no longer needed. I did list the charm proof statement there just to be complete. As you mention in the description the charm does not provide anything and that is by design so I believe that proof message is OK.

Thanks for your work making the openvpn charm better!

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

LGTM +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'config.yaml'
2--- config.yaml 2013-07-02 17:23:27 +0000
3+++ config.yaml 2014-01-24 02:23:20 +0000
4@@ -5,6 +5,7 @@
5 description: user for certificate generation
6 domain:
7 type: string
8+ default: ""
9 description: FQDN of the server, defaults to unit's given hostname
10 port:
11 type: int
12@@ -36,6 +37,7 @@
13 description: network to use for client addressing; must include a cidr suffix, i.e. X.X.X.X/XX
14 additional-routes:
15 type: string
16+ default: ""
17 description: additional routes to be routed through the VPN (i.e. '1.1.1.0/23, 2.2.2.0/8')
18 key-size:
19 type: int
20
21=== modified file 'metadata.yaml'
22--- metadata.yaml 2013-08-31 16:50:38 +0000
23+++ metadata.yaml 2014-01-24 02:23:20 +0000
24@@ -11,5 +11,4 @@
25 key exchange. It also supports VPNs with dynamic endpoints (DHCP or
26 dial-up clients), tunnels over NAT or connection-oriented stateful
27 firewalls (such as Linux's iptables).
28-categories:
29- - application
30+categories: ["cache-proxy"]
31
32=== removed file 'revision'
33--- revision 2013-08-31 16:50:38 +0000
34+++ revision 1970-01-01 00:00:00 +0000
35@@ -1,1 +0,0 @@
36-5

Subscribers

People subscribed via source and target branches

to all changes: