Thank you, Brad. The code is very clean, the tests are fast, and this is nice functionality.
The lurking issue you mentioned *may* be irrelevant when Francesco starts work on bug 188618. As I understand it, the config-changed hook is called initially, after install and before start. See Kapil's answer to http://askubuntu.com/questions/82683/what-juju-charm-hooks-are-available-and-what-does-each-one-do and the discussion of config-changed that he links to in https://juju.ubuntu.com/docs/drafts/service-config.html#creating-charms . This means that the install hook can sometimes be wildly simple: it should be able to defer all of its work to the config-changed hook. The only trick then, AFAIK, is that config-changed needs to be able to detect if things have been started yet, and not restart if things have started...or, it should embrace making the start hook irrelevant, perhaps, and always start, as you do now. Checking with Kapil or Ben or a charmer about my understanding and related charm best practices would be a reasonable step for all of these observations. To be honest, my big unhappiness and concern with this branch is that it is blocking Francesco starting cleanly on bug 1088618 tomorrow morning. That is something we must have to stay on schedule. We didn't need the config-changed functionality in order to meet the letter of the law, and as you know, I like to fulfill what we need and *then* back-fill the other bits that we have time for, if we still believe them to be important. I didn't communicate my position on this config-changed fix clearly in that regard, and it's my fault. There are a number of ways to resolve this, but under the circumstances, I suggest the following. Francesco should start work tomorrow morning with trunk, merged with this branch. He should feel free to make any changes to it that he needs, ignoring my discussion of possible simplification above unless he needs to address it for his goals. He can propose his branch with your branch as a dependency. You can pair with him to get his branch moving more quickly, if possible. Maybe in your morning you can resolve the answer to the question I raised at the top, and then in the afternoon he can pass the branch to you to take it farther after his EoD. Francesco, please communicate with Nicola about this situation in case he wants to start bug 1083920: as few changes to the charm as possible during your work will be helpful. Thanks again, Brad. This is good work. I want it to land, but there's some work to be done on it yet, and I don't want to have it at the expense of our schedule, so let's see if the coordination with Francesco that I propose might work. Gary -- https://code.launchpad.net/~bac/charms/precise/juju-gui/config-changed/+merge/140316 Your team Juju GUI Hackers is requested to review the proposed merge of lp:~bac/charms/precise/juju-gui/config-changed into lp:~juju-gui/charms/precise/juju-gui/trunk. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
