tacitknowledge / autopatch

An automated database patching framework for Java.
41 stars 19 forks source link

Change signature of MigrationRunnerStrategy.shouldMigrationRun #12

Closed MexicanHacker closed 12 years ago

MexicanHacker commented 12 years ago

The method MigrationRunnerStrategy.shouldMigrationRun should accept a PatchInfoStore object instead of an int (currentpatchlevel) to determine if a patch should run or not. This will help other strategies to use their own algorithm and not depend on the currentPatchLevel

hemri commented 12 years ago

I am wondering if the MigrationRunnerStrategy should have access to the PatchInfoStore object. This object permits to read useful information but also to modify sensible info directly into the patching system (e.g. PatchInfoStore.updatePatchLevel(int) ). Maybe we need to extract the key methods that are useful to the MigrationRunnerStrategy from PatchInfoStore to another interface. For key methods, I mean getPatchLevel and isPatchApplied(int).

ulisespulido commented 12 years ago

We need to create another interface and have PatchInfoStore extend this one, having the read only methods in there. What do you think about this?

MexicanHacker commented 12 years ago

Ulises,

I think we can separate this functionality in two, while I see the value on preventing the developers from updating the PatchInfoStore I think we can rely on them to be responsible until we attack this later. I would prefer to finish the functionality and attack the read only refactoring later, once we do a health assessment of the rest of the classes, as long as our code is tested and easy to refactor we should be fine.

ulisespulido commented 12 years ago

Signature changed and implemented interface changes to have read only operations for the different strategies, more read only methods with different information can be added later.

ulisespulido commented 12 years ago

A revert was done from the previous commit, we will handle the functionality separately.