Skip to content

Supply a Preconditions in common location#800

Merged
jonahgraham merged 1 commit into
mainfrom
pisv/preconditions
Feb 13, 2024
Merged

Supply a Preconditions in common location#800
jonahgraham merged 1 commit into
mainfrom
pisv/preconditions

Conversation

@pisv

@pisv pisv commented Feb 13, 2024

Copy link
Copy Markdown
Contributor

This is a follow-up to #798, which handles Preconditions similarly to ToStringBuilder.

See #742 for detailed discussion.

@pisv

pisv commented Feb 13, 2024

Copy link
Copy Markdown
Contributor Author

Note that #798 needs to be merged first.

@pisv pisv force-pushed the pisv/preconditions branch from f2ce823 to dbc2a66 Compare February 13, 2024 13:37
@jonahgraham

Copy link
Copy Markdown
Contributor

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 enableNullChecks provides by having multiple copies.

@jonahgraham

Copy link
Copy Markdown
Contributor

The documentation added in #799 needs to be updated too.

@jonahgraham

Copy link
Copy Markdown
Contributor

My alternate is in #801

This is a follow-up to #798, which handles `Preconditions` similarly to
`ToStringBuilder`.

See #742 for detailed discussion.
@jonahgraham

Copy link
Copy Markdown
Contributor

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.

@pisv

pisv commented Feb 13, 2024

Copy link
Copy Markdown
Contributor Author

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.

Makes sense to me, although both alternatives have their merits...

@jonahgraham

Copy link
Copy Markdown
Contributor

I have rebased and pushed. On successful build I will merge this.

@jonahgraham jonahgraham added this to the 0.22.0 milestone Feb 13, 2024
@jonahgraham jonahgraham merged commit 3002aa3 into main Feb 13, 2024
@jonahgraham jonahgraham deleted the pisv/preconditions branch February 13, 2024 15:01
@pisv

pisv commented Feb 13, 2024

Copy link
Copy Markdown
Contributor Author

The documentation added in #799 needs to be updated too.

Looks like I have completely missed that part, sorry... Unfortunately, I'll not be able to do it right now.

@jonahgraham

Copy link
Copy Markdown
Contributor

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.

@jonahgraham

Copy link
Copy Markdown
Contributor

I have the update on my other pr and will provide it soon.

Done in #804

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants