-
Notifications
You must be signed in to change notification settings - Fork 354
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 and simplify interface #320
Conversation
@shmax as its just a bitmask im ok with it, but we must avoid cloning/creating object at all costs |
Sounds good, I'll submit a PR soon. |
I went ahead and made the change in this PR. See what ya think... |
Go for it |
Already did! This PR is ready for review. |
Doesn't this change the interface again? It seems like these changes aren't BC with 4.0.0 and would force 5.0.0. |
Yes, I suppose it would. On the bright side, I don't think it's likely to inconvenience too many people--4.0 has only been out for a few days. |
@bighappyface Have you given any more thought to this PR? There are a few issues open right now that have to do with the documentation not matching the interface: |
First, let's rename this PR to be more descriptive. Second, I would like to get some additional community support for a move to 5.0.0 before I merge and go. I am not against it, but I am not a big fan of unilateral action for packages consumed by many people. |
Sure, no argument there. However, the community has had 2 weeks to chime in on this, and not a peep, so far. What's the best way to encourage some comment? |
Not as interested in this now that the flag for coercion has been removed. |
Updating the readme examples to reflect recent changes.
@digitalkaoz @bighappyface What do you guys think about the idea of moving checkmode back to constraint? It doesn't really seem to have anything to do with the optimizations, and it would make setting flags a little less awkward in the case that you don't want to otherwise inject any dependencies.