You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2022/01/13 00:25:49 UTC
[GitHub] [geode] pivotal-jbarrett opened a new pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
pivotal-jbarrett opened a new pull request #7262:
URL: https://github.com/apache/geode/pull/7262
<!-- Thank you for submitting a contribution to Apache Geode. -->
<!-- In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
-->
### 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](http://www.apache.org/legal/resolved.html#category-a)?
<!-- Note:
Please ensure that once the PR is submitted, check Concourse for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1012663789
This pull request **introduces 2 alerts** and **fixes 2** when merging 6b30ad018796c65d1d0eefe0b8bbedad60e5be50 into 93fdcccf19538562e932471c5711f26a295155ea - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-2862141f3787f948f380a38228c8bdc296de7a41)
**new alerts:**
* 2 for Type mismatch on container access
**fixed alerts:**
* 1 for Cast from abstract to concrete collection
* 1 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a change in pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #7262:
URL: https://github.com/apache/geode/pull/7262#discussion_r819192956
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/TXRemoteCommitMessage.java
##########
@@ -65,7 +65,8 @@ public static RemoteCommitResponse send(Cache cache, int txUniqId,
InternalDistributedMember onBehalfOfClientMember, DistributedMember recipient) {
final InternalDistributedSystem system =
(InternalDistributedSystem) cache.getDistributedSystem();
- final Set<DistributedMember> recipients = Collections.singleton(recipient);
+ final Set<InternalDistributedMember> recipients =
+ Collections.singleton((InternalDistributedMember) recipient);
Review comment:
We are inconsistent in our use of the interface vs the concrete class. I tried to keep the barrier as close to where the explicit cast was going to take place but it's complicated. In this case, all values in the set get blindly cast to `InternalDistributedMember`, I felt it was better to have the cast happen on the singular insert into the set and have the set properly typed for the usage. We need a better plan for internal interface/class casting overall.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1058812277
> A few nitpicks, but nothing that should hold up the PR, especially if there's a plan to do a second pass to refactor stuff.
Second, third, fourth... This should be an ongoing effort by everyone.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1011768900
This pull request **fixes 1 alert** when merging 6400b7452f02bdb40ee4152c8e9dffbc85a587f6 into ac04dc4348c9231960b46cefca13da41c1bff97e - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-92509c9a6f4f913a85ffc5e3b488776047076a94)
**fixed alerts:**
* 1 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1014720141
The 1 new LGTM alert, 'Inefficient output stream', is a false positive, reported https://github.com/github/codeql/issues/7610. LGTM suppression is also not working correctly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1013937746
This pull request **introduces 1 alert** and **fixes 8** when merging 3b5c06566eb6e8eb03651e1ae46b16c57895fd6e into 109fdd88a41d21c53f8f3da3b1dff5677afab3d2 - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-126721313d26a94a8ba4af3d647d32f4980fee84)
**new alerts:**
* 1 for Inefficient output stream
**fixed alerts:**
* 6 for Cast from abstract to concrete collection
* 2 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a change in pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #7262:
URL: https://github.com/apache/geode/pull/7262#discussion_r829564113
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/RegionAdvisor.java
##########
@@ -291,7 +291,7 @@ public Semaphore getVolunteeringSemaphore() {
}
}
if (logger.isDebugEnabled()) {
- logger.debug("This maps is sksk {} and size is {}", map, map.keySet().size());
+ logger.debug("This maps is sksk {} and size is {}", map, map.size());
Review comment:
Yeah, I don't have a clue. 🤷
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1013568581
This pull request **fixes 8 alerts** when merging 5628a5ccc95b9c3a693f5a50b75cf6c5a7f29a65 into 109fdd88a41d21c53f8f3da3b1dff5677afab3d2 - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-1f469f79c715b1b3c82552aeb8841d418c899605)
**fixed alerts:**
* 6 for Cast from abstract to concrete collection
* 2 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1059493936
Sorry for the rebase force push but our PR pipeline doesn't like merges... yet..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a change in pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #7262:
URL: https://github.com/apache/geode/pull/7262#discussion_r819191594
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRemoveAllOperation.java
##########
@@ -76,18 +78,18 @@
public int removeAllDataSize;
- protected boolean isBridgeOp = false;
+ protected boolean isBridgeOp;
static final byte USED_FAKE_EVENT_ID = 0x01;
static final byte NOTIFY_ONLY = 0x02;
static final byte FILTER_ROUTING = 0x04;
static final byte VERSION_TAG = 0x08;
static final byte POSDUP = 0x10;
static final byte PERSISTENT_TAG = 0x20;
- static final byte HAS_CALLBACKARG = 0x40;
+ // static final byte HAS_CALLBACKARG = 0x40;
Review comment:
See comment on documentation of unused protocol values.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1058835015
This pull request **introduces 1 alert** and **fixes 10** when merging b2e0b8e8304b1210d331f13dc8bfb1ecbd92d9bd into 7f5c883ad61912c14aace5ef950c3093ff06060b - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-24e23cd6803e0150e827b578f5327b639903939f)
**new alerts:**
* 1 for Inefficient output stream
**fixed alerts:**
* 8 for Cast from abstract to concrete collection
* 2 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] boglesby commented on a change in pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
boglesby commented on a change in pull request #7262:
URL: https://github.com/apache/geode/pull/7262#discussion_r829398717
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/RegionAdvisor.java
##########
@@ -291,7 +291,7 @@ public Semaphore getVolunteeringSemaphore() {
}
}
if (logger.isDebugEnabled()) {
- logger.debug("This maps is sksk {} and size is {}", map, map.keySet().size());
+ logger.debug("This maps is sksk {} and size is {}", map, map.size());
Review comment:
What is sksk? And `maps` should be `map`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1015586251
This pull request **introduces 1 alert** and **fixes 10** when merging c611f6721058c9d420fa5083364eaf8a962fbaf2 into 109fdd88a41d21c53f8f3da3b1dff5677afab3d2 - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-70781b34d9a0b4f3446cbcde89b848543283010d)
**new alerts:**
* 1 for Inefficient output stream
**fixed alerts:**
* 8 for Cast from abstract to concrete collection
* 2 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1053661520
This pull request **introduces 1 alert** and **fixes 10** when merging 7984bf3690c9c1cf48b81545c3a5dd98bc391b38 into b7297023c309210e70f3792980e75d10c54e7421 - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-24ac8470bbdd609970488d88bd06cfa9272cb161)
**new alerts:**
* 1 for Inefficient output stream
**fixed alerts:**
* 8 for Cast from abstract to concrete collection
* 2 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1051089421
This pull request **introduces 1 alert** and **fixes 10** when merging 31625c3128621609e95254062b9f7ed6039909f5 into 1fc35af2186e0b51a10347a4b84d45e1d96136a6 - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-4a0f1fb40b28672163580ce7de2bf89cf9bb6215)
**new alerts:**
* 1 for Inefficient output stream
**fixed alerts:**
* 8 for Cast from abstract to concrete collection
* 2 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a change in pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #7262:
URL: https://github.com/apache/geode/pull/7262#discussion_r819191456
##########
File path: geode-core/src/main/java/org/apache/geode/cache/query/internal/index/AbstractIndex.java
##########
@@ -283,9 +284,11 @@ public void query(Object key, int operator, Collection results, ExecutionContext
}
@Override
- public void query(Object key, int operator, Collection results, @Retained CompiledValue iterOp,
- RuntimeIterator indpndntItr, ExecutionContext context, List projAttrib,
- SelectResults intermediateResults, boolean isIntersection) throws TypeMismatchException,
+ public void query(Object key, int operator, Collection<Object> results,
+ @Retained CompiledValue iterOp,
+ RuntimeIterator runtimeIterator, ExecutionContext context, List<?> projAttrib,
Review comment:
My guess is that this was change as more of a cascading replacement. I didn't look to correct the poorly named symbols in all files I touched, just some of the more ugly ones in areas I was doing heavier work in.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a change in pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #7262:
URL: https://github.com/apache/geode/pull/7262#discussion_r819190801
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/DistributedPutAllOperation.java
##########
@@ -80,15 +80,15 @@
public int putAllDataSize;
- protected boolean isBridgeOp = false;
+ protected boolean isBridgeOp;
static final byte USED_FAKE_EVENT_ID = 0x01;
static final byte NOTIFY_ONLY = 0x02;
static final byte FILTER_ROUTING = 0x04;
static final byte VERSION_TAG = 0x08;
static final byte POSDUP = 0x10;
static final byte PERSISTENT_TAG = 0x20;
- static final byte HAS_CALLBACKARG = 0x40;
+ // static final byte HAS_CALLBACKARG = 0x40;
Review comment:
Good question. I left it in there not as commented out code but as documentation for what values we believe this protocol message to have but aren't utilizing. I could make that more clear by removing that type and modifiers I guess.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1038394883
This pull request **introduces 2 alerts** and **fixes 10** when merging bf0e4bc437b78f37ff4f32787c6b32bcbf9be14b into a98197b5d0a3a2547e0581e475dcabaa82e6e92f - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-522f647d18c4fcb905a072b3c83735b016be7495)
**new alerts:**
* 1 for Self assignment
* 1 for Inefficient output stream
**fixed alerts:**
* 8 for Cast from abstract to concrete collection
* 2 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1059531798
This pull request **introduces 1 alert** and **fixes 10** when merging 77c6d5c7b2a7171972efbc46b556d1660958ddb3 into ac9d1cf1906dbc38577f01fc3b60b255cafb1c67 - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-31f7567b9aaf5a55e836e2df9e9d05df818f90f6)
**new alerts:**
* 1 for Inefficient output stream
**fixed alerts:**
* 8 for Cast from abstract to concrete collection
* 2 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1058557696
> I feel conflicted about changes of this size and the fact that it would take me a very long time to properly review this.
It is either 1 PR with 100 commits or 100 PR with 1 commit. I think this approach should be faster. Unfortunately we chose to take human decision making out of our review process so all of you are required for review even though a few humans could have determined these are all analyzer cleanups.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1058554737
> If it's not too much hassle, it would be really helpful to have a list of the automatic clean-up tasks that were applied in this PR, to get an idea of what the scope of the changes are.
All of them. This PR follows a path through all the places we use Bucket Ids and cleans them up completely refactoring to come later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1012774023
This pull request **fixes 2 alerts** when merging 4a6eb28f12fb3258c2984748a01699335c1bd1c2 into 93fdcccf19538562e932471c5711f26a295155ea - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-747fc91db69e6f5f787954e2b33d9250bb999afa)
**fixed alerts:**
* 1 for Cast from abstract to concrete collection
* 1 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1015114374
This pull request **introduces 1 alert** and **fixes 10** when merging 4635fad0292867518329b8c2b925addbb9d63df3 into 109fdd88a41d21c53f8f3da3b1dff5677afab3d2 - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-34e8857495e6fbebe1697e75a60bdc2c816fb5bc)
**new alerts:**
* 1 for Inefficient output stream
**fixed alerts:**
* 8 for Cast from abstract to concrete collection
* 2 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1011610548
This pull request **fixes 1 alert** when merging 3dacbb7c37774ec3aa2ae689cbedb01a5891c3e7 into 83ce65f08db5d3d3e0f109dc8e532a998778ab21 - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-2bab65940cc0696f8b0dc0a18547fea47a9846e0)
**fixed alerts:**
* 1 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1038394883
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1038442693
This pull request **introduces 1 alert** and **fixes 10** when merging b1b7153449674fec014e65ee29752ba41a9250e1 into a98197b5d0a3a2547e0581e475dcabaa82e6e92f - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-195a680fa09a41cf15ec75256e49a1aa5ea346c1)
**new alerts:**
* 1 for Inefficient output stream
**fixed alerts:**
* 8 for Cast from abstract to concrete collection
* 2 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1013625086
This pull request **introduces 1 alert** and **fixes 8** when merging 5398d4fbcfe426fbaddd2f7ca4e984230773b9c9 into 109fdd88a41d21c53f8f3da3b1dff5677afab3d2 - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-ab9fcbc0326d8f185ae791e429e440e43f0aeaab)
**new alerts:**
* 1 for Inefficient output stream
**fixed alerts:**
* 6 for Cast from abstract to concrete collection
* 2 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a change in pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #7262:
URL: https://github.com/apache/geode/pull/7262#discussion_r829564113
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/RegionAdvisor.java
##########
@@ -291,7 +291,7 @@ public Semaphore getVolunteeringSemaphore() {
}
}
if (logger.isDebugEnabled()) {
- logger.debug("This maps is sksk {} and size is {}", map, map.keySet().size());
+ logger.debug("This maps is sksk {} and size is {}", map, map.size());
Review comment:
Yeah, I don't have a clue. 🤷
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] lgtm-com[bot] commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1012881703
This pull request **fixes 2 alerts** when merging 37a2d24eec06100c53bf765573b533efcb042b22 into 93fdcccf19538562e932471c5711f26a295155ea - [view on LGTM.com](https://lgtm.com/projects/g/apache/geode/rev/pr-d178ba4651a55f0d67827f8c92188f5f62956fca)
**fixed alerts:**
* 1 for Cast from abstract to concrete collection
* 1 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] boglesby commented on a change in pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
boglesby commented on a change in pull request #7262:
URL: https://github.com/apache/geode/pull/7262#discussion_r829398717
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/RegionAdvisor.java
##########
@@ -291,7 +291,7 @@ public Semaphore getVolunteeringSemaphore() {
}
}
if (logger.isDebugEnabled()) {
- logger.debug("This maps is sksk {} and size is {}", map, map.keySet().size());
+ logger.debug("This maps is sksk {} and size is {}", map, map.size());
Review comment:
What is sksk? And `maps` should be `map`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] DonalEvans commented on a change in pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
DonalEvans commented on a change in pull request #7262:
URL: https://github.com/apache/geode/pull/7262#discussion_r819030581
##########
File path: geode-core/src/main/java/org/apache/geode/cache/query/internal/index/RangeIndex.java
##########
@@ -1080,10 +1079,11 @@ private void addValuesToResultSingleKeyToRemove(Object entriesMap, Collection re
}
}
- private void addValuesToResult(final Object entriesMap, final Collection<?> result,
+ private void addValuesToResult(final Object entriesMap, final Collection<Object> result,
final Object keyToRemove, final CompiledValue iterOps, final RuntimeIterator runtimeItr,
Review comment:
For consistency, could `runtimeItr` be renamed "independentIterator" please?
##########
File path: geode-core/src/main/java/org/apache/geode/cache/query/internal/index/IndexProtocol.java
##########
@@ -96,13 +96,14 @@ void query(Object key, int operator, Collection results, Set keysToRemove,
* @param results The Collection object used for fetching the index results
* @param context ExecutionContext object
*/
- void query(Object key, int operator, Collection results, ExecutionContext context)
+ void query(Object key, int operator, Collection<Object> results, ExecutionContext context)
throws TypeMismatchException, FunctionDomainException, NameResolutionException,
QueryInvocationTargetException;
- void query(Object key, int operator, Collection results, CompiledValue iterOp,
- RuntimeIterator indpndntItr, ExecutionContext context, List projAttrib,
- SelectResults intermediateResults, boolean isIntersection) throws TypeMismatchException,
+ void query(Object key, int operator, Collection<Object> results, CompiledValue iterOp,
+ RuntimeIterator indpndntItr, ExecutionContext context, List<?> projAttrib,
Review comment:
For consistency with other changes in this PR, could `indpndntItr` be renamed "independentIterator" here?
##########
File path: geode-core/src/main/java/org/apache/geode/cache/query/internal/index/RangeIndex.java
##########
@@ -1109,9 +1109,10 @@ private void addValuesToResult(final Object entriesMap, final Collection<?> resu
}
private void addValuesToResultFromMap(final Map<Object, Object> entriesMap,
- final Collection<?> result, final Object keyToRemove, final CompiledValue iterOps,
+ final Collection<Object> result, final Object keyToRemove, final CompiledValue iterOps,
final RuntimeIterator runtimeItr, final ExecutionContext context, final List<?> projAttrib,
Review comment:
For consistency, could `runtimeItr` be renamed "independentIterator" please?
##########
File path: geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java
##########
@@ -242,11 +239,11 @@ public Object execute(Object[] params) throws FunctionDomainException, TypeMisma
// For local queries returning pdx objects wrap the resultset with
// ResultsCollectionPdxDeserializerWrapper
// which deserializes these pdx objects.
- if (needsPDXDeserializationWrapper(true /* is query on PR */)
+ if (needsPDXDeserializationWrapper( /* is query on PR */)
Review comment:
Small nitpick, but this comment and the similar one on line 284 can probably be removed, or at least moved outside the parentheses for the method call.
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/DistributedPutAllOperation.java
##########
@@ -80,15 +80,15 @@
public int putAllDataSize;
- protected boolean isBridgeOp = false;
+ protected boolean isBridgeOp;
static final byte USED_FAKE_EVENT_ID = 0x01;
static final byte NOTIFY_ONLY = 0x02;
static final byte FILTER_ROUTING = 0x04;
static final byte VERSION_TAG = 0x08;
static final byte POSDUP = 0x10;
static final byte PERSISTENT_TAG = 0x20;
- static final byte HAS_CALLBACKARG = 0x40;
+ // static final byte HAS_CALLBACKARG = 0x40;
Review comment:
Should this commented-out code be removed?
##########
File path: geode-core/src/main/java/org/apache/geode/cache/query/internal/index/AbstractIndex.java
##########
@@ -283,9 +284,11 @@ public void query(Object key, int operator, Collection results, ExecutionContext
}
@Override
- public void query(Object key, int operator, Collection results, @Retained CompiledValue iterOp,
- RuntimeIterator indpndntItr, ExecutionContext context, List projAttrib,
- SelectResults intermediateResults, boolean isIntersection) throws TypeMismatchException,
+ public void query(Object key, int operator, Collection<Object> results,
+ @Retained CompiledValue iterOp,
+ RuntimeIterator runtimeIterator, ExecutionContext context, List<?> projAttrib,
Review comment:
In `lockedQuery()` the variable name used for the `RuntimeIterator` in the method signature is "independentIterator". Also, in code that calls this `query()` method, the variable name is a variation of "independentIterator". Could it also be named "independentIterator" in this method signature for consistency?
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/TXRemoteCommitMessage.java
##########
@@ -65,7 +65,8 @@ public static RemoteCommitResponse send(Cache cache, int txUniqId,
InternalDistributedMember onBehalfOfClientMember, DistributedMember recipient) {
final InternalDistributedSystem system =
(InternalDistributedSystem) cache.getDistributedSystem();
- final Set<DistributedMember> recipients = Collections.singleton(recipient);
+ final Set<InternalDistributedMember> recipients =
+ Collections.singleton((InternalDistributedMember) recipient);
Review comment:
It's difficult to tell in a PR of this size, but is there a specific reason that this is being changed from the interface `DistributedMember` to the concrete class `InternalDistributedMember`?
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRemoveAllOperation.java
##########
@@ -76,18 +78,18 @@
public int removeAllDataSize;
- protected boolean isBridgeOp = false;
+ protected boolean isBridgeOp;
static final byte USED_FAKE_EVENT_ID = 0x01;
static final byte NOTIFY_ONLY = 0x02;
static final byte FILTER_ROUTING = 0x04;
static final byte VERSION_TAG = 0x08;
static final byte POSDUP = 0x10;
static final byte PERSISTENT_TAG = 0x20;
- static final byte HAS_CALLBACKARG = 0x40;
+ // static final byte HAS_CALLBACKARG = 0x40;
Review comment:
Should this commented-out code just be removed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a change in pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #7262:
URL: https://github.com/apache/geode/pull/7262#discussion_r819191796
##########
File path: geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java
##########
@@ -242,11 +239,11 @@ public Object execute(Object[] params) throws FunctionDomainException, TypeMisma
// For local queries returning pdx objects wrap the resultset with
// ResultsCollectionPdxDeserializerWrapper
// which deserializes these pdx objects.
- if (needsPDXDeserializationWrapper(true /* is query on PR */)
+ if (needsPDXDeserializationWrapper( /* is query on PR */)
Review comment:
Ha, good find.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] DonalEvans commented on pull request #7262: GEODE-6588: Cleanup static analyzer warnings.
Posted by GitBox <gi...@apache.org>.
DonalEvans commented on pull request #7262:
URL: https://github.com/apache/geode/pull/7262#issuecomment-1058416909
If it's not too much hassle, it would be really helpful to have a list of the automatic clean-up tasks that were applied in this PR, to get an idea of what the scope of the changes are.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org