-
Notifications
You must be signed in to change notification settings - Fork 44
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
Update readme #25
Comments
I think you're right that the README could use more examples. Could some of the test-cases be a basis for the examples? |
Yes, I think that would work pretty well :) Also the CLI should be documented (I was totally surprised that that existed ;) ). |
Yes, good call! |
Wondered about this today, but any reason for not following -i/-f semantics? I'd say -i should do -ask true, -f should do -ask false, and the default is up to you. |
Is that |
Ah sorry, I didn't read the updated readme yet. You're right of course. In that case --force is the option I'm missing ;) |
Actually, now I think about it, 'verify --ask false' is a strange name for 'diff'. When you call verify with ask I'd say you're not really verifying. What about
I might be missing something though, because I only use the default verifyer, and not the vimdiff, but I expect this would work the same in that case right? (on a sidenote, any reason you use -a in your vim-mapping? That's the default, right?) |
Yeah that's fair, but I don't like the syntax: $ approvals diff --diff vimdiff I agree that if we have an As for the third suggestion, I can imagine a use case where you would want to automatically accept the new output.
I think what I originally intended was a flag that is true if present, false if absent. Looking at it again, I totally screwed that up. |
approvals view? |
Oooh, I like that! |
I was browsing around the code and am amazed at the amount of functionality which I didn't see from the readme. I think it would be really worthwhile to update the readme with the functionality at least, even if it isn't very well written/with lots of examples for now. I really think this is a great gem, and it apparantly is even better than I thought :)
The text was updated successfully, but these errors were encountered: