You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "Akshat-Jain (via GitHub)" <gi...@apache.org> on 2024/12/12 07:49:59 UTC
[PR] Re-visit previously disabled spotbugs patterns and enable them (druid)
Akshat-Jain opened a new pull request, #17560:
URL: https://github.com/apache/druid/pull/17560
### Description
With #17466, we excluded a bunch of spotbugs patterns to avoid irrelevant changes to that PR, and decided to revisit them as a follow-up action item.
This PR re-visits all the spotbugs patterns we disabled at that time, and enables some of them. The ones which are still kept disabled are mostly either false alarms, or would require a wider scoped design change.
<hr>
This PR has:
- [x] been self-reviewed.
- [ ] added documentation for new or modified features or behaviors.
- [ ] a release note entry in the PR description.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Re-visit previously disabled spotbugs patterns and enable them (druid)
Posted by "Akshat-Jain (via GitHub)" <gi...@apache.org>.
Akshat-Jain commented on code in PR #17560:
URL: https://github.com/apache/druid/pull/17560#discussion_r1881798892
##########
processing/src/main/java/org/apache/druid/query/aggregation/Histogram.java:
##########
@@ -27,11 +27,11 @@
public class Histogram
{
- public float[] breaks;
- public long[] bins;
- public transient long count;
- public float min;
- public float max;
+ private float[] breaks;
+ protected long[] bins;
+ protected transient long count;
+ private float min;
+ private float max;
Review Comment:
Have made the change.
##########
extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java:
##########
@@ -41,8 +41,8 @@ public class ApproximateHistogram
// max size of the histogram (number of bincount/position pairs)
int size;
- public float[] positions;
- public long[] bins;
+ protected float[] positions;
+ private long[] bins;
Review Comment:
Have made the change.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Re-visit previously disabled spotbugs patterns and enable them (druid)
Posted by "kgyrtkirk (via GitHub)" <gi...@apache.org>.
kgyrtkirk merged PR #17560:
URL: https://github.com/apache/druid/pull/17560
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Re-visit previously disabled spotbugs patterns and enable them (druid)
Posted by "kgyrtkirk (via GitHub)" <gi...@apache.org>.
kgyrtkirk commented on code in PR #17560:
URL: https://github.com/apache/druid/pull/17560#discussion_r1881735031
##########
extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java:
##########
@@ -41,8 +41,8 @@ public class ApproximateHistogram
// max size of the histogram (number of bincount/position pairs)
int size;
- public float[] positions;
- public long[] bins;
+ protected float[] positions;
+ private long[] bins;
Review Comment:
why mix protected/private?
I don't see it wrong to have a pojo with open fields - but if thats not good; they should all be protected so that subclasses could do things
##########
processing/src/main/java/org/apache/druid/query/aggregation/Histogram.java:
##########
@@ -27,11 +27,11 @@
public class Histogram
{
- public float[] breaks;
- public long[] bins;
- public transient long count;
- public float min;
- public float max;
+ private float[] breaks;
+ protected long[] bins;
+ protected transient long count;
+ private float min;
+ private float max;
Review Comment:
protected
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org