META_CHANGES_R0

[dido]

META_CHANGES_R0

[meddle]

For a technical analysis, I think it passes...

3.5p (10m)

[meddle]

• Notes about the test:
  • Your test tests the logic for meta changes that are defined in the org.sophie2.base.model.resources.r4 module. The problem is that it is not in it and in a module that depends on it. I understand that you wanted to use the resources from the book model, but you could write a small hierarchy of dummy ones for this test.
  • The way you wrote the testSkipFirsRevison method it tested nothing but the assertEquals method of the Unit tests, I fixed that...
  • The names of your methods, for testing are connected with the book model too testSkipMoveChange instead testSkipInsignificantChange for example.
• As for the implementation up to now...
  • The revision's applyChange method is better to throw it's errors and every logic that applies a change to handle them differently, for now this implementation is working but real exceptions in the change logic will be hard to locate.
  • Similar problem is with MetaChange#skipRevision; run your test, you make assertations that are swallowed, your test for the first revision for example should catch the assertationerror thrown... I think that behavior is bad.
  • Your MetaChange needs factory methods like makeUndo(access), makeRedo(access), makeSkip(access, revison) or something like that, because writing meta changes with this big constructor is ugly...

So I think your design is good, but you were too lazy :) to write a book model independent test, so I'll pass it and you rewrite the test... As for the implementation, it will work like that, but you should talk with Milo and me what we will do with that throwable caching. For the implementation review write the factory methods of MetaChange.

3p (100m)

[meddle]

Umm other thing next time put the right revisions :)

[mira]

I fixed the pointed ishues and there is also a user logic for making undo and redo

[pap]

• In UndoRedoTest you could've used methods for creating pages and frames from ModelTest. This makes the test more readable.
• You could've added a method for creating a main window in AppTestBase. This is the way test helpers become useful.
• It is quite unnice to commit code with warnings.
• Fake JavaDoc is something very bad.
• Don't make meddle write the proper testing classes. Write the beforementioned yourself.
• UndoRedoLogic's logic is incorrect. You should not assert that there is a current BookDocView. Maybe you can check whether there is such and if not just don't do anything.
• Remember that there are other kinds of documents.
• The above even renders the enumeration name incorrect.
• Anyway we are very glad that we can perform such valuable actions at last.

[deyan]

Batch update from file query-obsoleted.csv