-
Notifications
You must be signed in to change notification settings - Fork 145
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
Supply a Preconditions in common location #800
Conversation
Note that #798 needs to be merged first. |
f2ce823
to
dbc2a66
Compare
I had another PR almost ready to go on this topic, but I didn't get it up before the end of the day. I was a little more weary of removing Preconditions because it is referenced from a number of places: https://github.com/search?q=Preconditions.enableNullChecks&type=code Do you think it is ok to have the breaking change that says means consumers need to change their imports? I assume that no one is really depending on the sort of scoping that the |
The documentation added in #799 needs to be updated too. |
My alternate is in #801 |
After more thought I want to go with this version and since there are already breaking changes in 0.22.0 compared to 0.21.0 I think we should remove rather than just deprecate the old Preconditions. @pisv before I rebase and merge this I will wait a little while for your comment. I am trying to complete the 0.22.0 today if possible. |
Makes sense to me, although both alternatives have their merits... |
dbc2a66
to
d9b5d22
Compare
I have rebased and pushed. On successful build I will merge this. |
Looks like I have completely missed that part, sorry... Unfortunately, I'll not be able to do it right now. |
No worries. I forgot to do it too. I miss Gerrit, it warns me before merging if there are unresolved comments. I have the update on my other pr and will provide it soon. |
Done in #804 |
This is a follow-up to #798, which handles
Preconditions
similarly toToStringBuilder
.See #742 for detailed discussion.