Need charm for quassel-core

[Thomi Richards]

Need a charm to deploy quassel-core.

[Thomi Richards (thomir-deactivatedaccount)]

This is my first charm - I'm sure I'm missing something, I'd love some pointers in the right direction!

[Mark Mims (mark-mims)]

reviewing now

[Mark Mims (mark-mims)]

Hi Thomi, thanks for contributing your charm!

need to fix:

- needs a copyright file at the root level of the charm... otherwise, people don't actually have license to use it. Just grab one from another charm and edit with your information

- please update metadata.yaml to include only the relations the service will actually provide. In this case, quassel-core requires nothing and is peer to nothing. It's safe to remove those fields. Not sure what variety of clients can connect to quassel-core, but perhaps the service could provide something like a "quassel-server" relation with a "quassel" interface? Thoughts?

- the `relation-name-xxx` hooks are still the empty templates. These hooks need to match the relations described in metadata.yaml. I'd recommend only keeping the `quassel-server-relation-changed` hook.

This looks like a great start... I'm excited to see a charm for the quassel server part!

Also, you might take a look at `charm proof` to check the charm out... it gives errors and warnings from static charm analysis

[Mark Mims (mark-mims)]

I'm leaving the new-charm tag, but changing status to "incomplete"... please update the branch and change this bug to "Fix Committed" when you're ready for another round of review.
Thanks!
Mark

[Paul Gear (paulgear)]

I discussed this with Thomi on IRC and I'm considering taking over this charm as a learning exercise, and because I'm a new Quassel fan. I'm working through the charm authoring guide (starting at https://juju.ubuntu.com/docs/authors-intro.html), but if there's anything specifically relevant to this charm that I should know about, please let me know.

[Paul Gear (paulgear)]

I've completed basic testing of a reasonably well-featured quassel-core charm. It works reasonably well for me.

The current version is available at https://code.launchpad.net/~paulgear/charms/precise/quassel-core/trunk

Despite the name precise, I've only tested it on trusty. Can anyone point to the correct process for setting up the correct charm distro for it?

[Paul Gear (paulgear)]

P.S. The charm should also work without issue on precise.

[Paul Gear (paulgear)]

I've conducted light testing on both precise and trusty and it works as expected.

[Paul Gear (paulgear)]

I've marked this as fix committed but I'm not sure what else needs to be done. I've had feedback from another user which indicates the charm is functional.

[Marco Ceppi (marcoceppi)]

Hi Paul,

Thanks for the update, it's now showing up in our review queue and I've requested a round of testing for this.

[Review Queue (review-queue)]

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

[Marco Ceppi (marcoceppi)]

Hi Paul,

There a few things which need to be addressed in this charm before it can move forward. The results of the test: http://reports.vapour.ws/charm-tests/charm-bundle-test-10853-results show the following issues with the `charm proof` output:

E: Charm must have either a maintainer or maintainers field

You're the author of the charm! Make sure you have the maintainers field in the metadata.yaml https://jujucharms.com/docs/authors-charm-metadata

W: Metadata missing required field "tags"

We've added a taxonomy of sorts to charms, add the tags field with a list of one or more tags that match this charm, https://jujucharms.com/docs/authors-charm-metadata

I: No icon.svg file.

This can be ignored for now, but consider making an icon using the icon template outlined in the docs: https://jujucharms.com/docs/authors-charm-icon

W: no README file

All charms require a README file, if you have the latest charm-tools package installed (https://jujucharms.com/docs/tools-charm-tools) you can just run `charm add readme` to get a boiler plate.

E: template interface names should be changed: interface-name
E: template relations should be renamed to fit charm: relation-name

Make sure to update the interfaces/relations for your charm in the metadata.yaml. If the charm has no relations, just remove the provides, requires, and peers section in the metadata.yaml

I: File config.yaml not found.

This can be ignored if you charm has no configuration options.

--

There are several files that aren't being used in the charm, mainly the relation files. If there aren't going to be any relations remove these files. There's no need to keep empty hook files around as Juju will work without them. The charm itself is very simple, and there's nothing wrong with that! As a first pass goes this is a great start!

Let us know if you have any questions

[Marco Ceppi (marcoceppi)]

Argh! Literally after hitting submit I realized the link branch was not yours, but the previous persons attempt. The testing results aren't accurate. I've linked your branch to this and re-requested tests. I'll wait for those results before weighing in again. Feel free to disregard the previous comment.

[Review Queue (review-queue)]

The results (PASS) are in and available here: http://reports.vapour.ws/charm-tests/charm-bundle-test-10863-results

[José Antonio Rey (jose)]

Hey Paul!

Thanks a bunch for submitting the quassel-core charm to the store. It is definitely a great addition! I am checking the branch, and turns out it doesn't have tests. I am currently working on them and will propose a merge against your branch, so please make sure to review that as soon as possible so I can do a review!

If you have any questions, please email us to <email address hidden> or find us on #juju on irc.freenode.net. Hoping to be able to review this soon!

[Charles Butler (lazypower)]

Greetings Paul,

Thanks for the contribution! The quassel-core charm is very close to being ready for inclusion in the charm store as a recommended charm.

I've taken a look over the work that's been done and I have some cursory notes:

There are missing tests, and this is a requirement for inclusion into the ~recommended namespace. Upon perusal of your repository I see some cursory tests (which just stand up the service) have been submit by Jose. This is a good starting place, as they were built with charm helpers [1] and offer a scaffolding template to get you moving with writing charm tests. `charm add tests` - will generate the same output.

What I'd like to see are a few cursory tests against the service to ensure it has done the following:

- Ensure the configuration file is actually updated wrt changing logging configuration
- Validate a SSL certificate has been generated
- Assert that the service is indeed running on the host (simple ps aux | grep quassel-core would be sufficient - or service status quassle-core)

With those three assertions, we can ensure the service will be resilient to future merges against the charm should someone else in the community wish to contribute to its development, and give us a nice baseline to gauge the health of the charm as time goes on.

Thanks again for the submission. I'm going to change status of this Bug to "In Progress" and when you're ready switch the bug status to 'Fix Committed' and someone will be along shortly to review your work.

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>, or ask a question tagged with "juju" on http://askubuntu.com.

Thanks again for your hard work and dedication on creating this charm - as its a brilliant service I use every day across multiple devices. Having it under juju orchestration would be a nice addition to my suite of personally-used-charms. And if you require any 1:1 assistance with the test writing, Jose and I are both available to assist.

[Paul Gear (paulgear)]

Thanks Charles, I'll work on getting up to speed with the testing framework soon. In the meantime, can I take your comments above as a +1 onhttps://code.launchpad.net/~jose/charms/precise/quassel-core/add-tests/+merge/246001? The reason it isn't integrated yet is because I didn't feel qualified to review it.

[Paul Gear (paulgear)]

I've created some basic amulet tests, which I think cover Charles' feedback above. I'm still waiting to see if they are viable. I assume they will appear at http://reports.vapour.ws/all-bundle-and-charm-results/lp%3A%7Epaulgear%252Fcharms%252Fprecise%252Fquassel-core%252Ftrunk ?

[Charles Butler (lazypower)]

Greetings @paulgear - that is correct. the test results will aggregate under that url. I'm taking a look at your amulet tests and should have some feedback shortly.

[Charles Butler (lazypower)]

A brief note about the tests,

the current layout with an init() method was causing testing woes. I created a fixup branch and proposed it for merging into yours. I apologize for not catching this in the proposed merge from Jose.

https://code.launchpad.net/~lazypower/charms/precise/quassel-core/update-50-deploy/+merge/255363

This should get you unblocked and moving @paulgear

Cheers!

[Charles Butler (lazypower)]

I've just circled back to check on the status of this bug, and see no changes from our last review. I'm going to move the status of this bug to WIP and when you're ready for a re-review simply move the bug status to fix-committed and someone will be along shortly to review your work.

Cheers

[Paul Gear (paulgear)]

Pushed an updated charm that ran to successful completion of amulet tests for me on Canonistack.

[Charles Butler (lazypower)]

Paul,

Thanks for your diligence on this charm. I've kicked the tires on the tests and can confirm it works as expected on all substrates. our CI infra structure confirms (aside from a few cloud based faux-paus during testing) - http://reports.vapour.ws/charm-test-details/charm-bundle-test-parent-244

I've promulgated the charm. It can be accessed in the store within ~ 60 minutes of this review. Please add the issue tracker to your notifications as the maintainer of this charm:

https://bugs.launchpad.net/charms/+source/quassel-core

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>, or ask a question tagged with "juju" on http://askubuntu.com.

[Paul Gear (paulgear)]

Thanks Charles; subscription added. What can we do about getting it published for trusty as well as precise?

[Charles Butler (lazypower)]

I'll do that now @paulgear, thanks for following up :)

[Charles Butler (lazypower)]

Allright, its been promulgated for trusty, and should show up in the store shortly.

https://launchpad.net/~charmers/charms/trusty/quassel-core/trunk

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>, or ask a question tagged with "juju" on http://askubuntu.com.