<div dir="ltr"><div>Hey Juju,</div><div> </div><div>Andrew, Kostas, Cory, and I made our way through a slew of reviews. It went something like this:</div><div> </div><div><ul style="margin-top:0pt;margin-bottom:0pt"><li style="list-style-type:disc;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">minecraft </li><ul style="margin-top:0pt;margin-bottom:0pt"><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent"><a href="https://bugs.launchpad.net/charms/+bug/1523896" style="text-decoration:none">https://bugs.launchpad.net/charms/+bug/1523896</a></li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">This is a rewrite of the precise minecraft charm, now using layers and targeting trusty.</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">We found a problem with the author’s ./trunk branch (possibly due to bzr artifacts between ‘charm build’ attempts), and suggested some test changes via a PR. The author addressed both of these issues and all looks good.</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">+1, promulgated, <a href="https://jujucharms.com/minecraft/trusty" style="text-decoration:none">https://jujucharms.com/minecraft/trusty</a> is live!</li></ul><li style="list-style-type:disc;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">rsyslog-forwarder-ha</li><ul style="margin-top:0pt;margin-bottom:0pt"><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent"><a href="https://code.launchpad.net/~johnsca/charms/trusty/rsyslog-forwarder-ha/lint/+merge/288469" style="text-decoration:none">https://code.launchpad.net/~johnsca/charms/trusty/rsyslog-forwarder-ha/lint/+merge/288469</a></li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Fixes lint issues</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Previously, ‘make test’ would pollute the system with a version of PyYAML that was incompatible with ‘charm proof’. The author has fixed this with tox.</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">+1, merged</li></ul><li style="list-style-type:disc;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">zulu8</li><ul style="margin-top:0pt;margin-bottom:0pt"><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent"><a href="https://bugs.launchpad.net/charms/+bug/1519858" style="text-decoration:none">https://bugs.launchpad.net/charms/+bug/1519858</a></li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">This is a layered bash charm that implements the java interface to provide a Zulu java environment.</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Testing was successful, and while we created an MP for minor tweaks (e.g. decreasing log noise), this didn’t warrant holding it up for further review.</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">+1, this is the first promulgated charm leveraging the java interface layer!</li></ul><li style="list-style-type:disc;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">rlec (new redis charm)</li><ul style="margin-top:0pt;margin-bottom:0pt"><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent"><a href="https://bugs.launchpad.net/charms/+bug/1551133" style="text-decoration:none">https://bugs.launchpad.net/charms/+bug/1551133</a></li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Missing crypto hash verification of the download</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Issues with handling of leadership and node registration</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Some erroneous boilerplate from `charm create` template</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">We also offered relation suggestions</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">-1 for now while the author considers these points</li></ul><li style="list-style-type:disc;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">haproxy (new statistics interface)</li><ul style="margin-top:0pt;margin-bottom:0pt"><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent"><a href="https://code.launchpad.net/~verterok/charms/trusty/haproxy/statistics-interface/+merge/288005" style="text-decoration:none">https://code.launchpad.net/~verterok/charms/trusty/haproxy/statistics-interface/+merge/288005</a></li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Bundletester (still) works fine, make build with new test works too. However, I manually deployed and related to telegraf and couldn’t see any hooks fired related to statistics so have asked the author for instructions to test further…</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Author replied almost immediately and suggested I test against a different telegraf MR - it did and it worked, so +1 for merging.</li></ul><li style="list-style-type:disc;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Apache2 (timeouts fix)</li><ul style="margin-top:0pt;margin-bottom:0pt"><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent"><a href="https://code.launchpad.net/~kwmonroe/charms/trusty/apache2/timeouts/+merge/287222" style="text-decoration:none">https://code.launchpad.net/~kwmonroe/charms/trusty/apache2/timeouts/+merge/287222</a></li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Adding some slack in the unit deployment timeout.</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Tests pass.</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">+1 for merging</li></ul><li style="list-style-type:disc;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Apache2 (ensure_package_status fix) </li><ul style="margin-top:0pt;margin-bottom:0pt"><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent"><a href="https://code.launchpad.net/~lutostag/charms/trusty/apache2/fix-extra-package-hold/+merge/287224" style="text-decoration:none">https://code.launchpad.net/~lutostag/charms/trusty/apache2/fix-extra-package-hold/+merge/287224</a></li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">This fix is for a check on extra package installations that was producing a warning.</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">We reviewed the code and run a couple of tests.</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">+1, merged</li></ul></ul><ul style="margin-top:0pt;margin-bottom:0pt"><li style="list-style-type:disc;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">fiche (new charm)</li><ul style="margin-top:0pt;margin-bottom:0pt"><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent"><a href="https://bugs.launchpad.net/charms/+bug/1530976" style="text-decoration:none">https://bugs.launchpad.net/charms/+bug/1530976</a> </li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">The author addressed our comments. However we asked for the layer that is used to build the charm.</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Adam Israel jumped in to finish the review and promulgate (thanks Adam!)</li></ul><li style="list-style-type:disc;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">midonet-gateway (new charm)</li><ul style="margin-top:0pt;margin-bottom:0pt"><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent"><a href="https://bugs.launchpad.net/charms/+bug/1541735" style="text-decoration:none">https://bugs.launchpad.net/charms/+bug/1541735</a></li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Review items addressed</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Unable to run tests, but James offered to do so and give final sign off.</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Otherwise, looks good from review. +1</li></ul><li style="list-style-type:disc;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">collectd (new charm)</li><ul style="margin-top:0pt;margin-bottom:0pt"><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent"><a href="https://bugs.launchpad.net/charms/+bug/1538573" style="text-decoration:none">https://bugs.launchpad.net/charms/+bug/1538573</a> and layer at <a href="https://code.launchpad.net/~jacekn/canonical-is-charms/collectd-composer" style="text-decoration:none">https://code.launchpad.net/~jacekn/canonical-is-charms/collectd-composer</a></li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">This is the third iteration over the collectd charm, and we appreciate the author’s dedication to addressing our comments!</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">Unfortunately, we spotted a small typo in the templating section that needs to be fixed. We also had some suggestions to improve the README. We submitted an MP to address both of these issues.</li><li style="list-style-type:circle;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;background-color:transparent">We’ll wait for the author’s input on our MP, but feel this is very close to promulgation.</li></ul></ul> </div><div>Thanks for all the submissions - we found lots of great new additions during this review. Questions or comments? Let us know in #juju on freenode. If we're not playing minecraft, we'll be right with you :)</div><div> </div><div>Thanks,</div><div>-Kevin Monroe</div></div>