Skip to content

Conversation

@davisusanibar
Copy link
Contributor

@davisusanibar davisusanibar commented Jan 20, 2024

Rationale for this change

To enable code review and formatting code by using Spotless Maven plugin.

What changes are included in this PR?

  • Maven configuration to enable Spotless Maven plugin.
  • Documentation how to use ´mvn spotless:apply´

Are these changes tested?

By ´mvn spotless:apply´

Are there any user-facing changes?

No

@github-actions
Copy link

⚠️ GitHub issue #39712 has been automatically assigned in GitHub to PR creator.

@github-actions github-actions bot added awaiting review Awaiting review Component: Java and removed awaiting review Awaiting review labels Jan 20, 2024
@davisusanibar
Copy link
Contributor Author

Please consider: Initial formatting implementations will require a large number of file changes.

Among the main changes that need to be reviewed are:

  • pom.xml
  • maven/pom.xml
  • bom/pom.xml

@vibhatha
Copy link
Contributor

@davisusanibar shall we hold this merge until we merge the errorprone fixes?
Then it will help a lot to clean up everything at once?

@davisusanibar
Copy link
Contributor Author

@davisusanibar shall we hold this merge until we merge the errorprone fixes? Then it will help a lot to clean up everything at once?

Hi @vibhatha, this sounds like a good idea to me.

@github-actions github-actions bot added the awaiting review Awaiting review label Jan 29, 2024
@davisusanibar
Copy link
Contributor Author

The documentation has been added, please review if more details need to be added.

CC: @vibhatha @lidavidm @danepitkin

@davisusanibar
Copy link
Contributor Author

davisusanibar commented Jan 30, 2024

The documentation has been added, please review if more details need to be added.

CC: @vibhatha @lidavidm @danepitkin

File at: https://github.com/apache/arrow/blob/a0194a2cd145beb8f845604b36fe982500ec888e/docs/source/developers/java/development.rst#code-style

@lidavidm
Copy link
Member

(1) did you mean to leave this in draft
(2) is it perhaps possible to enable this one module at a time?

@davisusanibar
Copy link
Contributor Author

(1) did you mean to leave this in draft (

Our plan was to merge first #39777 #39713 (comment)

  1. is it perhaps possible to enable this one module at a time?

Let me break this down into modules.

Comment on lines +31 to +42
* <p>CHAR --> ArrowType.Utf8 NCHAR --> ArrowType.Utf8 VARCHAR --> ArrowType.Utf8 NVARCHAR -->
* ArrowType.Utf8 LONGVARCHAR --> ArrowType.Utf8 LONGNVARCHAR --> ArrowType.Utf8 NUMERIC -->
* ArrowType.Decimal(precision, scale) DECIMAL --> ArrowType.Decimal(precision, scale) BIT -->
* ArrowType.Bool TINYINT --> ArrowType.Int(8, signed) SMALLINT --> ArrowType.Int(16, signed)
* INTEGER --> ArrowType.Int(32, signed) BIGINT --> ArrowType.Int(64, signed) REAL -->
* ArrowType.FloatingPoint(FloatingPointPrecision.SINGLE) FLOAT -->
* ArrowType.FloatingPoint(FloatingPointPrecision.SINGLE) DOUBLE -->
* ArrowType.FloatingPoint(FloatingPointPrecision.DOUBLE) BINARY --> ArrowType.Binary VARBINARY -->
* ArrowType.Binary LONGVARBINARY --> ArrowType.Binary DATE --> ArrowType.Date(DateUnit.MILLISECOND)
* TIME --> ArrowType.Time(TimeUnit.MILLISECOND, 32) TIMESTAMP -->
* ArrowType.Timestamp(TimeUnit.MILLISECOND, timezone=null) CLOB --> ArrowType.Utf8 BLOB -->
* ArrowType.Binary
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's an example of a change we wouldn't want. There might be a few of these types of issues around.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguably, the original formatting is wrong (javadoc would have rendered it wrong too) and this should've been an HTML definition list

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But yes, it'll be easier to review if this is broken down

@github-actions github-actions bot added awaiting committer review Awaiting committer review awaiting changes Awaiting changes and removed awaiting review Awaiting review awaiting committer review Awaiting committer review labels Jan 30, 2024
@davisusanibar
Copy link
Contributor Author

In order to define a better alternative solution, the current pull request has been closed

@vibhatha vibhatha mentioned this pull request Mar 23, 2024
15 tasks
@vibhatha
Copy link
Contributor

In order to define a better alternative solution, the current pull request has been closed

Created an issue: #40757

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Java] Enable code review and formatting code through Spotless Maven plugin

4 participants