-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cleanup and delete multiple snapshots at once (#bsc#956143). #39
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.
Convert.convert(new, :from => "map", :to => "map <string, any>") | ||
) { |key, value| ret = ret || Ops.get(orig, key) != value } | ||
ret | ||
new.map do |k,v| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
map? it is not good one. Better use each
and the best ruby is probably:
new.any? { |k, v| orig[k] != v }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Yast.import "Snapper" | ||
|
||
|
||
describe "Yast::Snapper" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here with object
in general nice improvement, just some code parts needs improvement. For tests I know it is just move, but if you touch it, it can be improved. The majority of comments are nitpick, but some of it should be addressed (especially map overusage). |
This bug will be fixed in PR #40. |
Not merge if more tests coverage is needed. I will be working on it and also in refacor/cleanup code.