[WIP] Added snapshot Class with strategy pattern. #40
Conversation
- Unified SnapperDbus and Snapper test in one file - Added tests for Snapper.ReadConfigs - Added tests for SnapperDialogsInclude#snapshot_modified
- Spliting some big methods trying to isolate its behavior to be easier for read, understood and rewrite.
| # ------------------------------------------------------------------------------ | ||
|
|
||
| require "yast" | ||
| require_relative "snapshot_dbus" |
There was a problem hiding this comment.
It breaks e.g. y2update ability, better is to use require "snapper/snapshot_dbus"
| ) { |key, value| ret = ret || Ops.get(orig, key) != value } | ||
| ret | ||
| def snapshot_modified(orig, args) | ||
| args.any? { |k, v| orig.send(k) != v } |
There was a problem hiding this comment.
I personally prefer using public_send as it do not discard encapsulation
|
in general nice to have some tests there ( maybe enabling coveralls shows how is status here ). Idea in general looks good, just please test code enough, so we are sure that nothing is broken. |
|
This looks as a big refactoring PR which will still need a lot of time. @teclator create a Trello card so this can be properly planned in some next SPRINT. |
|
@lslezak I will do |
|
It needs some love... and changes according to last snapper updates which probably makes the addition of different strategies not so useful. More details in this trello card: https://trello.com/c/Y4EcAhOn/1365-yast2-snapper-finish-cleanup |
I want to discuss the idea before do more work that could go directly to the trash :P. It's just a preview so please don't be picky about details in the code.
Cleaning up yast-snapper, i missed a more oriented way of use snapshots, so i was planing to add a Snapshot class and rewrite Snapper to work with Snapshot objects instead of hashes.
I asked my doubts in the channel and Imobach told me that in yast-yast2 already exist a similar class. This class is currently used for installation proposes and when DBus is still not available so Ancorgs suggested the use of this class but rewriting it to use the Strategy pattern, so we could have to strategies one for CLI and other for DBus
So what i really do was write a class that could be merged in FsSnapshot that use by default the SnapshotDBus strategy.
So the plan would be remove SnapperDbus and rewrite Snapper to maintain the actual published methods but calling FsSnapshot directly but being only responsable of the dialog logic.