| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 44
Upgrade Guava 33.4.0-jre -> 33.4.7-jre #1598
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
Conversation
|
Suggested commit message: |
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
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.
Hmm, added a few requireNonNulls. Not sure what we want as an approach. Require to have an extra message parameter to give a nicer message?
Before doing so everywhere, do we agree on the approach? I couldn't find a way around the Iterables.getOnlyElement nullability, while it is a bit strange to me that we now need to handle it like this becuase of a "ParameterizedNullness"...
I also added the -XepOpt:NullAway:JSpecifyMode=true, can move it to a different PR if desired, but it didn't have much effect it seems.
Edited/Blocked NotificationRenovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR. You can manually request rebase by checking the rebase/retry box above. |
rickie
force-pushed
the
renovate/guava-33.x
branch
from
1bf0db0 to
2ba0d38
Compare
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
Stephan202
force-pushed
the
renovate/guava-33.x
branch
from
2ba0d38 to
de3e283
Compare
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
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.
Rebased and added a commit.
Looks like both NullAway and IntelliJ IDEA do not interpret the @ParametricNullness annotations as expected. Adding -XepOpt:NullAway:JSpecifyMode=true doesn't improve the situation, but does flag additional things. I moved those changes to a separate PR: #1608.
CC @msridhar and @cpovirk; what are your thoughts on the newly introduced warnings? (Just checking: I assume that this is unrelated?)
|
This is a little embarrassing, but, uh, where do I see the warnings? :) |
|
@cpovirk ah, they're now all suppressed; see the changes with accompanying (Let me try to produce a full overview in a follow-up comment; sec.) |
|
To reproduce: Output: |
|
Ha, thanks, [the pointer to look for XXX in the PR is] all I needed :) |
|
Looks good. All 1 mutations in this change were killed.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
|
I can't recall what NullAway does with |
|
If I point error-prone-support at an identical Guava but with all the Here was a previous discussion about NullAway and My guess is that NullAway is still doing the best it can with |
And right, I don't think that's related. |
|
I think NullAway should ignore |
|
FYI NullAway 0.12.5 included the change to ignore |
Stephan202
force-pushed
the
renovate/guava-33.x
branch
from
59c8147 to
bd7ba6f
Compare
Stephan202
changed the base branch from
master
to
sschroevers/guava-33.4.5-upgrade-with-jspecify-mode
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
Stephan202
changed the title
|
Looks good. No mutations were possible for these changes. |
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.
I rebased the PR on top of #1608 and bumped to version to 33.4.6. I resolved conflicts simply by dropping a bunch of commits; sorry about that. With JSpecify mode enabled, the upgrade now requires no changes, beyond the JSR-305 workaround described in uber/NullAway#1171. For that I ported the relevant changes from #1608, but with updated comments.
Also here: thanks for the rapid feedback and help @cpovirk and @msridhar!
rickie
force-pushed
the
sschroevers/guava-33.4.5-upgrade-with-jspecify-mode
branch
from
60d9f56 to
6ef1c6e
Compare
Stephan202
force-pushed
the
renovate/version.nullaway
branch
from
aeb9587 to
14a1ee1
Compare
Stephan202
force-pushed
the
renovate/version.nullaway
branch
3 times, most recently
from
a1dd062 to
cb8b432
Compare
Stephan202
force-pushed
the
renovate/guava-33.x
branch
from
8b402f7 to
103e241
Compare
Stephan202
changed the title
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.
Bumped to 33.4.7. The PR now targets master and is ready to merge.
|
Looks good. No mutations were possible for these changes. |
1 similar comment
|
Looks good. No mutations were possible for these changes. |
|
This PR contains the following updates:
33.4.0-jre->33.4.5-jre33.4.0-jre->33.4.5-jre