eod status 27-jun-2012

William Reade william.reade at canonical.com
Wed Jun 27 15:58:24 UTC 2012


Hi all

I have one *significant* branch out for review:

* https://codereview.appspot.com/6347045/

It's much less big than it looks, but it rather guts state/watcher.go by
moving the common features of the various content watchers into a single
type embedded by all the others. I feel that it is now *far* easier to
understand the operation of the individual watcher types [0]; as
evidence for this, I would like to observe that it was only after making
the change that it became clear that most of the watchers just don't
bother checking whether a given content change actually represents a
real change in state.

I personally feel those issues should be fixed, because it'll make it a
lot easier for the clients if they can assume that changes really are
changes; opinions?

It also brings watcher usage in cmd/jujud in line with other instances,
but those changes are so trivial by comparison that they didn't seem
worth splitting out into a separate CL.

The presence.ChildrenWatcher has been reparented to that branch, because
it now makes use of the moved stopWatcher/mustErr; I consider this
change to be worthwhile, because we have a track record of getting this
behaviour subtly wrong. New CL, with all known issues addressed, at:

* https://codereview.appspot.com/6354045/

Separately, the addition of a Version field to ContentChange, as
discussed last week, is frankly trivial, and is at:

* https://codereview.appspot.com/6330053/

...while the ServiceRelationsWatcher, which will want to be reworked in
the light of the big branch, but should still be reviewable, is at:

* https://codereview.appspot.com/6338059/

I need to be off very shortly, but hope to pop back on tonight to
coincide with niemeyer; I would *hugely* appreciate it if he would take
a look at the big one sometime earlyish in his day, in the hope that I
can get a quick turnaround on the first bunch of changes tonight.

Once I know where I'm going with these I'll take the RelatedUnitsWatcher
off hiatus (which depends on all branches above except the last one);
and should (once the fourth branch is also done) then be able to close
off a decent chunk of the unit agent work.

Ofc, if other considerations intrude, I have plenty of interesting bugs
related to deploy that I can work on tomorrow, so don't feel too
pressured :).

Cheers
William

[0] "Three strikes and you refactor" may be arguable, but I think it's
harder to deny the wisdom of refactoring after 7 ;). There's surely
space for argument over the precise form of the refactoring, though :).




More information about the Juju-dev mailing list