New vagrant pristine command
#5613
Conversation
| DESC | ||
|
|
||
| command("pristine") do | ||
| require File.expand_path("../command", __FILE__) |
There was a problem hiding this comment.
Let's use require_relative instead please
|
Hey @fgrehm Thank you for the Pull Request. At a high level, this looks really great. Do you think you could add some tests for this? If you look at the pushes tests, I think you can find a good pattern to use. I'm just really hesitant to merge such a large change without test coverage. Even if it works now, we need to prevent regressions in the future 馃槃 |
|
Sure! I'll try to write those tests over the week. |
|
Thanks for this. I'm not a big fan of the name, I don't know what to expect with |
|
.@sethvargo is trying to convince me this should be a command and not a flag. We'll delay this for 1.8 in some form. |
|
No worries! I'm not sure when I'll have a chance to write tests for this so feel free to close the PR if you guys want to hold back on it! 馃嵒 |
|
Hey @fgrehm Thanks for the PR. Coming around back to this (almost a year later), I think I've had a change of heart. After reading through all the comments in the original issue, I think everyone's opinions on "what this should be named" and "what it should do" are too far apart. As such, I'm going to recommend folks use a shell alias to involve whatever their preferred behavior is, instead of building this into core. I totally appreciate your efforts on this PR, but I'm actively working to simplify Vagrant's CLI API, and I don't think it's wise to add new commands at this time, especially since this "works" with some |
|
No need to be sorry! 馃嵒 |
Closes GH-5378
Ideally we should just reuse the other command plugins instead of copying and pasting but its been a while since I been around the codebase and I can't remember if it would be possible to do that 馃槉
/cc @sethvargo