I took my own advice.. And reviewed one of the existing stories. I have a bunch of comments below.
Reviewing the Create Consumer Code ==================================
ConsumerResource ---------------- - I assume the owner creation code in the ctor should be a fixme. - Should the Identity cert creation go into the Curator? This seems like business logic. - In general, many of these classes actually implement business logic (much of the bind logic). Should this move into a lower level business curator? Perhaps there are other interim curators like the entitler. Although the binder is probably a bad name :) - Any reason to not rip out the commented out code blocks for the old methods?
IdentityCertAdapter ------------------- - Currently, the adapter needs to (1) Generate and (2) Persist the new certficate in the IdentityCertificate database. Do we want the candlepin engine proper to actually do the persisting? It seems like this is a little bit too much bleed over across the API.
Default Identity Certificate ---------------------------- - nextSerialNumber has a bit of a race condition :) - why is username in the DN?
-- bk