ARROW-4741: [Java] Add missing type javadoc and enable checkstyle by emkornfield · Pull Request #4374 · apache/arrow

emkornfield · 2019-05-23T06:30:44Z

This change:

Enables a check style rule on all public, protected and package visible classes/enums.
Adds missing javadocs to make the rule pass
Adds some private constructors to static method utility classes (some of these are little bit short, and in some cases especially flight some of the purposes of classes was a littler hard to infer).
Cleans up redundant public qualifier on interfaces when found.

- Also make private constructors to static utility method classes as they were encountered. - Remove redundant public declarions in interfaces.

java/flight/src/main/java/org/apache/arrow/flight/CallOptions.java

java/flight/src/main/java/org/apache/arrow/flight/FlightService.java

praveenbingo · 2019-05-23T12:20:06Z

java/flight/src/main/java/org/apache/arrow/flight/NoOpFlightProducer.java

 import org.apache.arrow.flight.impl.Flight.PutResult;

+/**
+ * A {@link FlightProducer} that throws on all operations.


i am not sure why a noop producer throws on all operations :) should we change this (in another jira probably?)

It could be changed later, but the Javadoc is fine. (It's just meant to provide default implementations - guess no-op is misleading for what it does though.)

java/flight/src/main/java/org/apache/arrow/flight/Result.java

java/vector/src/main/java/org/apache/arrow/vector/types/IntervalUnit.java

emkornfield

@praveenbingo thank you for the review I think I addressed all your comments.

java/flight/src/main/java/org/apache/arrow/flight/CallOptions.java

java/flight/src/main/java/org/apache/arrow/flight/FlightService.java

emkornfield · 2019-05-24T04:34:02Z

java/flight/src/main/java/org/apache/arrow/flight/NoOpFlightProducer.java

 import org.apache.arrow.flight.impl.Flight.PutResult;

+/**
+ * A {@link FlightProducer} that throws on all operations.


java/flight/src/main/java/org/apache/arrow/flight/Result.java

java/vector/src/main/java/org/apache/arrow/vector/types/IntervalUnit.java

praveenbingo · 2019-05-24T06:34:06Z

@emkornfield
Thanks a ton for doing this, couple gandiva classes looks like are failing with some checkstyle issues. Once we fix, we should be good to go.

emkornfield · 2019-05-26T05:29:41Z

@praveenbingo Build is green.

emkornfield · 2019-05-28T05:00:38Z

@BryanCutler did you want to take a look before I merge?

praveenbingo

@emkornfield Thanks a ton for doing this. LGTM.

BryanCutler · 2019-05-28T17:55:47Z

java/flight/src/main/java/org/apache/arrow/flight/GenericOperation.java

 package org.apache.arrow.flight;

+/**
+ * Unused?.


I'm not sure what this is for and I don't see where it is used either, maybe @jacques-n can comment?

BryanCutler

LGTM, thanks for doing this

BryanCutler · 2019-05-28T18:04:33Z

Merged to master

This change: 1. Enables a check style rule on all public, protected and package visible classes/enums. 2. Adds missing javadocs to make the rule pass 3. Adds some private constructors to static method utility classes (some of these are little bit short, and in some cases especially flight some of the purposes of classes was a littler hard to infer). 4. Cleans up redundant public qualifier on interfaces when found. Author: Micah Kornfield <emkornfield@gmail.com> Closes apache#4374 from emkornfield/class_javadoc and squashes the following commits: 10ff97a <Micah Kornfield> fix last gandiva issue cb3175b <Micah Kornfield> Fix gandiva docs 32a4a18 <Micah Kornfield> address review feedback a64f658 <Micah Kornfield> the rest of the classes 3e3d6d6 <Micah Kornfield> ARROW-4741: Add type javadoc.

emkornfield added 2 commits May 22, 2019 23:17

ARROW-4741: Add type javadoc.

3e3d6d6

- Also make private constructors to static utility method classes as they were encountered. - Remove redundant public declarions in interfaces.

the rest of the classes

a64f658

praveenbingo reviewed May 23, 2019

View reviewed changes

address review feedback

32a4a18

emkornfield commented May 24, 2019

View reviewed changes

emkornfield self-assigned this May 24, 2019

emkornfield added 2 commits May 25, 2019 19:21

Fix gandiva docs

cb3175b

fix last gandiva issue

10ff97a

praveenbingo approved these changes May 28, 2019

View reviewed changes

BryanCutler reviewed May 28, 2019

View reviewed changes

BryanCutler approved these changes May 28, 2019

View reviewed changes

BryanCutler changed the title ~~ARROW-4741: [Java] Add missing type javadoc and enable checkstyle~~ ARROW-4741: [Java] Add missing type javadoc and enable checkstyle May 28, 2019

BryanCutler closed this in 9fe728c May 28, 2019

asfimport mentioned this pull request May 28, 2019

[Java] Add documentation to all classes and enable checkstyle for class javadocs #21261

Closed

2 tasks

Conversation

emkornfield commented May 23, 2019

Uh oh!

Uh oh!

Uh oh!

praveenbingo May 23, 2019

Choose a reason for hiding this comment

Uh oh!

emkornfield May 24, 2019

Choose a reason for hiding this comment

Uh oh!

lidavidm May 24, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

emkornfield left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

emkornfield May 24, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

praveenbingo commented May 24, 2019

Uh oh!

emkornfield commented May 26, 2019

Uh oh!

emkornfield commented May 28, 2019

Uh oh!

praveenbingo left a comment

Choose a reason for hiding this comment

Uh oh!

BryanCutler May 28, 2019

Choose a reason for hiding this comment

Uh oh!

BryanCutler left a comment

Choose a reason for hiding this comment

Uh oh!

BryanCutler commented May 28, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants