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
GEODE-10261: VMProvider.invokeAsync uses appropriate parameterization. #7631
Conversation
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.
Great clean-up! At the risk of adding more work, if you do an IntelliJ search for AsyncInvocation[^<;]
with case sensitivity and regex matching enabled and filter the results to "Except Comments and String Literals", you can find all the places in the codebase that AsyncInvocation
is used without parameterization. There only seem to be about 20 classes left, so with a bit more work, you could eliminate all of the places where this class is used without parameterization.
...a/org/apache/geode/cache/query/internal/index/InitializeIndexEntryDestroyQueryDUnitTest.java
Outdated
Show resolved
Hide resolved
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PutAllGlobalDUnitTest.java
Outdated
Show resolved
Hide resolved
...he/geode/internal/cache/partitioned/PersistentColocatedPartitionedRegionDistributedTest.java
Outdated
Show resolved
Hide resolved
geode-core/src/distributedTest/java/org/apache/geode/management/DiskManagementDUnitTest.java
Outdated
Show resolved
Hide resolved
...e-dunit/src/distributedTest/java/org/apache/geode/test/dunit/tests/BasicDistributedTest.java
Outdated
Show resolved
Hide resolved
geode-dunit/src/distributedTest/java/org/apache/geode/test/dunit/tests/VMDistributedTest.java
Outdated
Show resolved
Hide resolved
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.
Since this appears to be a mostly automated refactor I am not going to review every file but the quick look is great. Thanks for cleaning up all these warnings!!!
39f2ce5
to
4115723
Compare
...e-dunit/src/distributedTest/java/org/apache/geode/test/dunit/tests/BasicDistributedTest.java
Outdated
Show resolved
Hide resolved
8364a87
to
c077a23
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.
Looks good. If you can, please update the description with a little more detail on what you changed. For example:
Add generic parameters to AsyncInvocation variables where they were missing.
Cleaned up places where there was array of size 1 when it could have just been a variable.
Thanks!
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.
Approving the changes in geode-dunit
* Cleanup use of AsyncInvocation. * Add generic parameters to AsyncInvocation variables where they were missing. * Cleaned up by changing single element arrays to variables.
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
Has your PR been rebased against the latest commit within the target branch (typically
develop
)?Is your initial contribution a single, squashed commit?
Does
gradlew build
run cleanly?Have you written or updated unit tests to verify your changes?
If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?