-
Notifications
You must be signed in to change notification settings - Fork 170
Conversation
5a47ef9
to
10b85c5
Compare
reset! | ||
|
||
unless data.kind_of?(Hash) | ||
raise InvalidUndoRecord, "Undo data is incorrectly formatted. Must be a Hash, got '#{data}'." |
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.
Do you want to display data
or data.class
?
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.
I want to display data
because it might help you figure out what went wrong. But now that I think of it, this should probably be data.inspect
🚀 |
|
||
class UndoStack | ||
|
||
MAX_SIZE = 10 |
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.
10
seems kind of low, and it also seems like users may want to make this configurable. What about the use case of never deleting the backups off disc?
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.
The use case I'm trying to address is the "oh crap I didn't mean to delete that thing" or "oops I thought no one was using that" kind of thing. Think of this as an alternative to having a confirmation dialog (e.g., "are you sure you want to delete X?"). Since it's just stored locally on your laptop, it won't be useful to other folks on your team, so I don't think it can serve a generalized backup function (need to use DB snapshots or knife EC backup instead).
Make sense?
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.
Sure, that makes sense. I was thinking about if there was an automated system that removed old policies, but as you say, this tool is mean to be used from a workstation. 👍
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.
Yeah, here's the design work I did before writing this: https://gist.github.com/danielsdeleo/1723952c9175a3e80c10
Basically, I see 2 kinds of interaction models. For cookbook artifacts and policy revisions, it's totally possible to determine which ones are not in use and clean them up via a garbage collection strategy. In those cases you don't really need a backup or undo (I think) because you know they're unused. For policy groups, I don't think we can come up with a solid and universal way to distinguish "garbage" from "non-garbage," so you need a human to say "delete this thing." But humans make mistakes so we want undo functionality to help humans recover from mistakes.
One question, but otherwise 👍 |
10b85c5
to
3dc88b7
Compare
Relies on APIs that don't exist yet, so it won't actually work. That will be addressed soon, then I'll come back and add CLIs on top of this.
Also adds an undo stack so that we don't need to have a naggy confirmation dialog thing