-
Notifications
You must be signed in to change notification settings - Fork 125
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
Remove backup
and check
options
#546
Conversation
6ed37cc
to
c849c46
Compare
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.
As per #529 (comment), I'd revert the removal of backup
before merging this. Otherwise, I think it's good to go, great work! 🎉
The removal of the check
option has been discussed on the related linked issue and I think there is consensus for removing it, given that its functionality can be achieved by cleverly combining other options.
c849c46
to
fa9bddb
Compare
@AlexTMjugador Updated with backup removal notice: |
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.
Sweet, I think we can merge this then. Great work!
Tidy up the API by removing a couple of options we don't really need. Backup was discussed in shssoichiro#542 Check was discussed in shssoichiro#439 @shssoichiro Just say if you prefer to keep either of these 🙂
Tidy up the API by removing a couple of options we don't really need.
Backup was discussed in #542
Check was discussed in #439
@shssoichiro Just say if you prefer to keep either of these 🙂