Code review comment for lp:~canonical-ci-engineering/ubuntu-ci-services-itself/pm

Overall: sounds great but already a bit complex, I wonder if we can simplify things a bit.

Some details:

24 +* Runs as a persistent service, when it goes down, the engine halts.
25 +* State needs to serialized whenever it is updated, so that status can be regenerated on a restart.

+1

57 +* PPA Assigner - Provides two PPAs to perform package builds, one for MPs themselves, another for MPs + trunk.

Isn't phase 0 using a single PPA only ?

60 +* Integration Test Runner - Runs the specified tests on up to 3 images.
61 +

Just 'Test Runner', the 'Integration' is silent ;)

80 + package: chris-foobar-pkg
81 + version: 1.1-0ubuntu2
82 + source: chris-foobar-src

Is there a strong reason to use ':' or can we use '=' instead ?

I don't want to bikeshed there so if you feel strongly about, go for
it. Otherwise, this is configuration data and I'd rather use '=' to make
sure we keep it simple and don't turn it into a full json that only experts
can decipher ;)

233 + WORKFLOW_STEPS = (
239 + WORKFLOW_STEP_STATUSES = (

I don't get why those are split, aren't them all just states for the ticket ?

1 === added file 'docs/components/houston.rst'
380 === removed file 'docs/components/project-manager.rst'

O.M.G. !

This is a sin, please use 'bzr mv docs/components/project-manger.rst docs/components/houston.rst' !

A kitten dies everytime you do remove+add ;-)

More seriously, bzr supports renames, use that feature, it preserves
history, in this specific case it would have more clearly show what you did
change in this proposal.

....

Ha right, that was a rewrite on the side and the remove was revno 27.

Ok, the kitten survived after all ;)

Summary, no need to block on the questions asked above, the answers can come in a followup, this is significant progress, let's land it.