I am currently the SPOF (single point of failure) with Bosco, as it turns out, keeping an eye out in case I miss something. But a single point of failure seems preferable to sending the reports to the whole dev list NO ONE taking responsibility.
The best strategy, in fact, is for all contributors to take responsibility for quality. (Committers need to take responsibility for their own work AND the work of the contributor that they are committing.) Run your own favorite quality checks (your IDE, findbugs) and you’ll find bugs in your own code and maybe someone else’s. If we all run the same checks we won’t get as much coverage. Julian > On Dec 3, 2015, at 3:57 PM, Josh Elser <[email protected]> wrote: > > Yeah, I have already registered, Bosco. > > I'd rather not become a SPOF though. If we send to the list, the load can be > spread (these kinds of fixes are great intro-tasks for people). > > Don Bosco Durai wrote: >> Josher >> >> If you are a committer in Calcite, then you can register yourself at >> https://scan.coverity.com/projects/apache-calcite?tab=overview. The scans >> are done twice a week and if there are any issues found, then the email is >> automatically sent. >> >> Bosco >> >> >> >> >> >> On 12/3/15, 3:21 PM, "Josh Elser"<[email protected]> wrote: >> >>> Alright, will do. Thanks for bringing it up. >>> >>> Do we want to just set up Coverity so that we get these emails directly? >>> >>> Julian Hyde wrote: >>>> Bosco, >>>> >>>> Yes, this is a problem. Thanks for forwarding. >>>> >>>> Josh, >>>> >>>> You made this change yesterday. Can you please fix? >>>> >>>> I see quite a few examples in that file that should be boilerplate. Why >>>> does >>>> >>>> if (obj == null || !(obj instanceof RpcMetadataResponse)) { >>>> return false; >>>> } >>>> >>>> depart from >>>> >>>> if (!(obj instanceof RpcMetadataResponse)) { >>>> return false; >>>> } >>>> >>>> which is sufficient and is used elsewhere in the file? Boring stuff >>>> should look boring. >>>> >>>> In fact the whole method could be simplified. Rather than >>>> >>>> public boolean equals(Object obj) { >>>> if (this == obj) { >>>> return true; >>>> } >>>> >>>> if (obj == null || !(obj instanceof RpcMetadataResponse)) { >>>> return false; >>>> } >>>> >>>> RpcMetadataResponse other = (RpcMetadataResponse) obj; >>>> if (serverAddress == null) { >>>> if (other.serverAddress != null) { >>>> return false; >>>> } >>>> } >>>> return serverAddress.equals(other.serverAddress); >>>> } >>>> >>>> there is a case to be made that >>>> >>>> public boolean equals(Object obj) { >>>> return this == obj >>>> || obj instanceof RpcMetadataResponse >>>> && Objects.equals(((RpcMetadataResponse) obj).serverAddress, >>>> serverAddress); >>>> } >>>> >>>> is more understandable as well as more concise. >>>> >>>> Elsewhere in the file there is -- not written by you -- the following: >>>> >>>> if (null == connectionId) { >>>> if (null != other.connectionId) { >>>> return false; >>>> } >>>> } else if (!connectionId.equals(other.connectionId)) { >>>> return false; >>>> } >>>> >>>> return true; >>>> >>>> It works, but not obviously so. >>>> >>>> Julian >>>> >>>> >>>> On Thu, Dec 3, 2015 at 11:21 AM, Bosco Durai<[email protected]> >>>> wrote: >>>>>> 3195 return serverAddress.equals(other.serverAddress); >>>>> Good catch. I know you are monitoring it. Just wanted double sure about >>>>> it… >>>>> >>>>> Thanks >>>>> >>>>> Bosco >>>>> >>>>> >>>>> >>>>> >>>>> On 12/3/15, 11:16 AM, "[email protected]"<[email protected]> >>>>> wrote: >>>>> >>>>>> Hi, >>>>>> >>>>>> Please find the latest report on new defect(s) introduced to Apache >>>>>> Calcite found with Coverity Scan. >>>>>> >>>>>> 1 new defect(s) introduced to Apache Calcite found with Coverity Scan. >>>>>> >>>>>> >>>>>> New defect(s) Reported-by: Coverity Scan >>>>>> Showing 1 of 1 defect(s) >>>>>> >>>>>> >>>>>> ** CID 120367: Null pointer dereferences (FORWARD_NULL) >>>>>> /avatica/src/main/java/org/apache/calcite/avatica/remote/Service.java: >>>>>> 3195 in >>>>>> org.apache.calcite.avatica.remote.Service$RpcMetadataResponse.equals(java.lang.Object)() >>>>>> >>>>>> >>>>>> ________________________________________________________________________________________________________ >>>>>> *** CID 120367: Null pointer dereferences (FORWARD_NULL) >>>>>> /avatica/src/main/java/org/apache/calcite/avatica/remote/Service.java: >>>>>> 3195 in >>>>>> org.apache.calcite.avatica.remote.Service$RpcMetadataResponse.equals(java.lang.Object)() >>>>>> 3189 RpcMetadataResponse other = (RpcMetadataResponse) obj; >>>>>> 3190 if (serverAddress == null) { >>>>>> 3191 if (other.serverAddress != null) { >>>>>> 3192 return false; >>>>>> 3193 } >>>>>> 3194 } >>>>>>>>> CID 120367: Null pointer dereferences (FORWARD_NULL) >>>>>>>>> Calling a method on null object "serverAddress". >>>>>> 3195 return serverAddress.equals(other.serverAddress); >>>>>> 3196 } >>>>>> 3197 } >>>>>> 3198 } >>>>>> 3199 >>>>>> >>>>>> >>>>>> ________________________________________________________________________________________________________ >>>>>> To view the defects in Coverity Scan visit, >>>>>> https://scan.coverity.com/projects/apache-calcite?tab=overview >>>>>> >>>>>> To manage Coverity Scan email notifications for "[email protected]", >>>>>> click >>>>>> https://scan.coverity.com/subscriptions/edit?email=bosco%40apache.org&token=368c97cfe2c99af46cce4f2657f16fe3 >>>>>> >>
