Re: Some minor migration changes I had lying around
oh...@google.com
http://codereview.waveprotocol.org/605001/diff/1/2
File src/org/waveprotocol/wave/migration/DocumentHistoryMigrator.java
(right):
http://codereview.waveprotocol.org/605001/diff/1/2#newcode1
Line 1: // Copyright 2011 Google Inc. All Rights Reserved.
please use the apache license header
http://codereview.waveprotocol.org/605001/diff/1/2#newcode6
Line 6: import com.google.gwt.dev.util.Preconditions;
wrong preconditions? could use the one from model, or from
com.google.common.base since this file also depends on collect anyway.
http://codereview.waveprotocol.org/605001/diff/1/2#newcode32
Line 32: this.decorators = ImmutableList.copyOf(decorators);
It's not pretty to add setTarget() to NindoCursorDecorator and to use it
here to mutate the arguments destructively. One alternative would be to
take a Function<NindoCursor, NindoCursor> (that builds a chain of
decorators given the target NindoCursor) as the argument here and call
it in line 49 with the target Nindo.Builder.
Or, even better, just to take a Function<Nindo, Nindo> that replaces
lines 46-53 and let the caller of this class use whatever mechanism they
want to convert the nindos. We could add other utilities to define a
Function<Nindo, Nindo> in terms of decorators and Nindo.Builder, but
that is orthogonal to this class.
http://codereview.waveprotocol.org/605001/diff/1/2#newcode38
Line 38: public DocOp migrate(DocOp old) throws OperationException {
Maybe rename to migrateAndApply() to make it clearer how it interacts
with getCurrentState().
http://codereview.waveprotocol.org/605001/diff/1/4
File
src/org/waveprotocol/wave/model/document/operation/NindoCursorDecorator.java
(right):
http://codereview.waveprotocol.org/605001/diff/1/4#newcode20
Line 20: import com.google.gwt.dev.util.Preconditions;
wrong preconditions?