© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
a60fb26
(Get the code!)
Hello, Brad!
I've done a review of the bip charm for trusty. Let's get started!
# Overview
I took a quick look at the charm and, at a first glance, I just notice that there's a default username and password. This could be a security risk, since it's publicly available. Do you think it's possible to make the charm configuration fail in case there's no username/password, and then retry once it gets one? Anyways, let's get this deployed both with Amulet and manually.
# Deployment (manual)
I did the deployment as specified on the README.md file and had absolutely no problems. It installed fine, and I could connect over my IRC client.
# Deployment (CI-like)
I ran the tests using bundletester and passed successfully, no errors.
# Others
As Amir pointed out, it would be nice if the README.md file was expanded a bit more. If I didn't know what bip was, I wouldn't understand what the charm was for. Apart from that, everything looks to be ready for promulgation.
# Conclusion
Thanks for working on the bip charm, Brad! I hope to see the next iteration of the bip charm really soon. If you fix what I suggested earlier, I believe it would be ready to go to the store.
I will move the status of this bug to "Incomplete". Please, move it to either "New" or "Fix Committed" once the charm is ready for review again. Thanks again for submitting the charm!