Code review comment for lp:~divmod-dev/divmod.org/obtain-2824-6

Revision history for this message
Jean-Paul Calderone (exarkun) wrote :

Okay. Branch is too large. It is a challenge to understand (either the change or just the new code on its own). Too large partly because it does too many things - adds better naming features, updates unrelated documentation, changes how actions are dispatched, changes the action class hierarchy naming, etc. Hard to understand because the simulation core now hinges on a number of distinct concepts - links, annotations, obstructions, retrievers, maybe some others - but lacks any documentation about how they relate to each other (Thing.__doc__ is a noble effort, but gives up about 1/6th of the way though).

I think near future efforts on Imaginary need to focus on documentation, examples, and test coverage. Once the branch is merged, I hope efforts will pick up on those fronts.

review: Approve

« Back to merge proposal