You are viewing a plain text version of this content. The canonical link for it is here.
Posted to devnull@infra.apache.org by "joan38 (GitHub)" <gi...@apache.org> on 2018/08/23 18:43:15 UTC
[GitHub] [kafka] joan38 opened pull request #5562: Make
FunctionConversions private
As pointed out in this comment https://github.com/apache/kafka/pull/5539#discussion_r212380648 the object `FunctionConversions` is only of internal use and therefore should be private to the lib only so that we can do changes without going threw KIP like this one.
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] joan38 commented on issue #5562: KIP-363 Make
FunctionConversions deprecated
Posted by "joan38 (GitHub)" <gi...@apache.org>.
Thanks @guozhangwang
I get:
```
Bug type EQ_UNUSUAL (click for details) In class org.apache.kafka.streams.scala.FunctionsCompatConversions$AggregatorFromFunctionIn method org.apache.kafka.streams.scala.FunctionsCompatConversions$AggregatorFromFunction.equals(Object)At FunctionsCompatConversions.scala:[line 88]
--
```
```
EQ_UNUSUAL: Unusual equals method
This class doesn't do any of the patterns we recognize for checking that the type of the argument is compatible with the type of the this object. There might not be anything wrong with this code, but it is worth reviewing.
```
<img width="1434" alt="screen shot 2018-09-17 at 08 40 18" src="https://user-images.githubusercontent.com/1200478/45610858-5cfef780-ba55-11e8-9d9c-80bbf9dcdaf7.png">
Is it possible to disable this?
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] guozhangwang closed pull request #5562: KIP-363 Make
FunctionConversions deprecated
Posted by "guozhangwang (GitHub)" <gi...@apache.org>.
[ pull request closed by guozhangwang ]
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] joan38 commented on issue #5562: KIP-363 Make
FunctionConversions deprecated
Posted by "joan38 (GitHub)" <gi...@apache.org>.
Thanks @guozhangwang
I get:
```
Bug type EQ_UNUSUAL (click for details) In class org.apache.kafka.streams.scala.FunctionsCompatConversions$AggregatorFromFunctionIn method org.apache.kafka.streams.scala.FunctionsCompatConversions$AggregatorFromFunction.equals(Object)At FunctionsCompatConversions.scala:[line 88]
--
```
```
EQ_UNUSUAL: Unusual equals method
This class doesn't do any of the patterns we recognize for checking that the type of the argument is compatible with the type of the this object. There might not be anything wrong with this code, but it is worth reviewing.
```
<img width="1434" alt="screen shot 2018-09-17 at 08 40 18" src="https://user-images.githubusercontent.com/1200478/45610858-5cfef780-ba55-11e8-9d9c-80bbf9dcdaf7.png">
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] joan38 commented on issue #5562: KIP-363 Make
FunctionConversions private
Posted by "joan38 (GitHub)" <gi...@apache.org>.
@guozhangwang This is all good to merge
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] joan38 commented on issue #5562: KIP-363 Make
FunctionConversions deprecated
Posted by "joan38 (GitHub)" <gi...@apache.org>.
@vvcephei you are right, it is excluded.
I will add this one also to the exclusion.
Other than that are we happy with the change?
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] vvcephei commented on issue #5562: KIP-363 Make
FunctionConversions deprecated
Posted by "vvcephei (GitHub)" <gi...@apache.org>.
@joan38 . Hmm. I'm not sure why it's running for that one class. Perhaps the other class is already excluded, and this one doesn't match the pattern?
The exclusions are here: `gradle/spotbugs-exclude.xml`
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] joan38 commented on issue #5562: KIP-363 Make
FunctionConversions deprecated
Posted by "joan38 (GitHub)" <gi...@apache.org>.
It's now ready to merge
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] joan38 commented on issue #5562: KIP-363 Make
FunctionConversions private
Posted by "joan38 (GitHub)" <gi...@apache.org>.
We are waiting for the min of 72h of the KIP vote to finish:
http://mail-archives.apache.org/mod_mbox/kafka-dev/201808.mbox/%3CCAJYfrs3GOrFrY9K3wToyg+Pj-AXtc=RZvvCWTH9fmWVjsEYs_w@mail.gmail.com%3E
Which means we can merge this after 26 Aug 2018 18:24:56 GMT
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] joan38 commented on issue #5562: KIP-363 Make
FunctionConversions deprecated
Posted by "joan38 (GitHub)" <gi...@apache.org>.
Thanks @guozhangwang
I get:
```
Bug type EQ_UNUSUAL (click for details) In class org.apache.kafka.streams.scala.FunctionsCompatConversions$AggregatorFromFunctionIn method org.apache.kafka.streams.scala.FunctionsCompatConversions$AggregatorFromFunction.equals(Object)At FunctionsCompatConversions.scala:[line 88]
--
```
```
EQ_UNUSUAL: Unusual equals method
This class doesn't do any of the patterns we recognize for checking that the type of the argument is compatible with the type of the this object. There might not be anything wrong with this code, but it is worth reviewing.
```
<img width="1434" alt="screen shot 2018-09-17 at 08 40 18" src="https://user-images.githubusercontent.com/1200478/45610858-5cfef780-ba55-11e8-9d9c-80bbf9dcdaf7.png">
Is it possible to disable this since it's a copy and past from the other class?
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] joan38 commented on issue #5562: KIP-363 Make
FunctionConversions deprecated
Posted by "joan38 (GitHub)" <gi...@apache.org>.
retest this please
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] mjsax commented on issue #5562: KIP-363 Make
FunctionConversions private
Posted by "mjsax (GitHub)" <gi...@apache.org>.
@joan38 I just replied to the KIP thread -- also note, that the vote did not pass yet and the KIP is not accepted yet. It's required to have 3 binding votes (so far only Guozhang gave binding +1).
Also, can you update this PR accordingly? Thanks a lot.
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] guozhangwang commented on issue #5562: KIP-363 Make
FunctionConversions deprecated
Posted by "guozhangwang (GitHub)" <gi...@apache.org>.
@joan38 spotbugs was added recently, you can run `./gradlew spotbugsMain spotbugsTest -x test` (stated in `README.md`) locally to see the results.
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] debasishg commented on issue #5562: KIP-363 Make
FunctionConversions private
Posted by "debasishg (GitHub)" <gi...@apache.org>.
👍
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] joan38 commented on issue #5562: KIP-363 Make
FunctionConversions deprecated
Posted by "joan38 (GitHub)" <gi...@apache.org>.
```
12:40:15 * What went wrong:
12:40:15 Execution failed for task ':streams:streams-scala:spotbugsMain'.
12:40:15 > SpotBugs rule violations were found. See the report at: file:///home/jenkins/jenkins-slave/workspace/kafka-pr-jdk10-scala2.12/streams/streams-scala/build/reports/spotbugs/main.xml
```
Do you know what this means?
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] joan38 commented on issue #5562: KIP-363 Make
FunctionConversions private
Posted by "joan38 (GitHub)" <gi...@apache.org>.
We are waiting for the min of 72h of the KIP vote to finish:
http://mail-archives.apache.org/mod_mbox/kafka-dev/201808.mbox/%3CCAJYfrs3GOrFrY9K3wToyg+Pj-AXtc=RZvvCWTH9fmWVjsEYs_w@mail.gmail.com%3E
Which means we can merge this on 26 Aug 2018 18:24:56 GMT
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] joan38 commented on issue #5562: KIP-363 Make
FunctionConversions deprecated
Posted by "joan38 (GitHub)" <gi...@apache.org>.
retest this please
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] guozhangwang commented on issue #5562: KIP-363 Make
FunctionConversions deprecated
Posted by "guozhangwang (GitHub)" <gi...@apache.org>.
spotbug still fails in jenkins:
```
04:43:17 Execution failed for task ':streams:streams-scala:spotbugsMain'.
```
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org
[GitHub] [kafka] joan38 commented on issue #5562: KIP-363 Make
FunctionConversions private
Posted by "joan38 (GitHub)" <gi...@apache.org>.
We are waiting for the min of 72h of the KIP vote to finish:
http://mail-archives.apache.org/mod_mbox/kafka-dev/201808.mbox/%3CCAJYfrs3GOrFrY9K3wToyg+Pj-AXtc=RZvvCWTH9fmWVjsEYs_w@mail.gmail.com%3E
Which means we can merge this after 26 Aug 2018 18:24:56 GMT
Can we label this `streams` @mjsax ?
Thanks
[ Full content available at: https://github.com/apache/kafka/pull/5562 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org