Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-41173: [Java] Add spotless configuration for Maven pom.xml files #41174

Merged
merged 3 commits into from
Apr 17, 2024

Conversation

laurentgo
Copy link
Collaborator

@laurentgo laurentgo commented Apr 12, 2024

What changes are included in this PR?

To make sure that all pom.xml files follow the same convention regarding indentation and organization, add spotless plugin with sortPom module to check and format the files automatically.

Update Java developer documentation to provide the link to the convention and instructions on how to use spotless

Finally apply the convention to all pom.xml files

Are these changes tested?

No new test as no code change (just formatting)

Are there any user-facing changes?

None

@laurentgo laurentgo requested a review from lidavidm as a code owner April 12, 2024 20:29
Copy link

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

In the case of PARQUET issues on JIRA the title also supports:

PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

See also:

…iles

To make sure that all pom.xml files follow the same convention regarding
indentation and organization, add spotless plugin with sortPom module to
check and format the files automatically.

Update Java developer documentation to provide the link to the
convention and instructions on how to use spotless

*Note* this change does not include changes to pom.xml files (and would
fail the build. Changes to pom.xml files are in the next commit.
Apply spotless formatter to all pom.xml files
@laurentgo laurentgo force-pushed the laurentgo/GH-41173/maven-pom-format branch from 5167672 to 8fbda83 Compare April 12, 2024 20:31
@laurentgo laurentgo changed the title Laurentgo/gh 41173/maven pom format GH-41173: [Java] Add spotless configuration for Maven pom.xml files Apr 12, 2024
Copy link

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

Downgrate spotless to 2.30.0 (last java 8 release)
@vibhatha
Copy link
Collaborator

@laurentgo there is already and effort to add this module by module: #40757

@laurentgo
Copy link
Collaborator Author

laurentgo commented Apr 15, 2024

I saw the spotless change but I assumed (wrongly) it was for Java files, not pom.xml. My bad.

I would argue that pom.xml have a style on their own, and that there's only a few of them (compared to the numerous java files).

At the same time, as someone who's offering/trying to clean up those files, it's kind of hard to have to match each file own style...

@lidavidm
Copy link
Member

I think we can do this first. @vibhatha would it conflict?

@vibhatha
Copy link
Collaborator

This is formatting all poms. I thought we wanted a smaller diff as a previous PR was closed for the length of it.

If this solves the problem, let's close the issues created per module.

Also I would recommend to check existing issues before creating new ones.

@vibhatha
Copy link
Collaborator

I saw the spotless change but I assumed (wrongly) it was for Java files, not pom.xml. My bad.

I would argue that pom.xml have a style on their own, and that there's only a few of them (compared to the numerous java files).

At the same time, as someone who's offering/trying to clean up those files, it's kind of hard to have to match each file own style...

It's fine. Let's try to wrap up this PR.
But let's close the other tickets if it closes the expected via this PR.

Objective is to format code and xml both.

If this PR is only doing xml format we can declare that and make code changes in the other list.

Also please feel free to work on those issues if it's in the scope.

@lidavidm
Copy link
Member

The previous PR formatted nearly everything.

Copy link
Collaborator

@vibhatha vibhatha left a comment

Choose a reason for hiding this comment

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

Looks fine to me. Let's verify the CIs.

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Apr 17, 2024
@vibhatha
Copy link
Collaborator

The previous PR formatted nearly everything.

Yes, except the Java. I looked into the changes. I am not an expert with spotless, but this only formats the pom.

It would be very hard to be sure about all formatting. I will run the crossbows and validate. That should be one validator.

I think we decided to go module by module since it's hard to eye-check everything in a single PR.

@vibhatha
Copy link
Collaborator

But this PR is manageable. We can use the other spotless tickets to format code and licenses.

@vibhatha
Copy link
Collaborator

@github-actions crossbow submit -g java

Copy link

Revision: 590d4e9

Submitted crossbow builds: ursacomputing/crossbow @ actions-c929748329

Task Status
java-jars GitHub Actions
test-conda-python-3.10-spark-v3.5.0 GitHub Actions
test-conda-python-3.11-spark-master GitHub Actions
test-conda-python-3.8-spark-v3.5.0 GitHub Actions
verify-rc-source-java-linux-almalinux-8-amd64 GitHub Actions
verify-rc-source-java-linux-conda-latest-amd64 GitHub Actions
verify-rc-source-java-linux-ubuntu-20.04-amd64 GitHub Actions
verify-rc-source-java-linux-ubuntu-22.04-amd64 GitHub Actions
verify-rc-source-java-macos-amd64 GitHub Actions

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels Apr 17, 2024
@lidavidm lidavidm merged commit 7003e90 into apache:main Apr 17, 2024
18 checks passed
@lidavidm lidavidm removed the awaiting merge Awaiting merge label Apr 17, 2024
Copy link

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 7003e90.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 8 possible false positives for unstable benchmarks that are known to sometimes produce them.

tolleybot pushed a commit to tmct/arrow that referenced this pull request May 2, 2024
…iles (apache#41174)

### What changes are included in this PR?

To make sure that all pom.xml files follow the same convention regarding indentation and organization, add spotless plugin with sortPom module to  check and format the files automatically.
    
Update Java developer documentation to provide the link to the  convention and instructions on how to use spotless
    
Finally apply the convention to all `pom.xml` files

### Are these changes tested?

No new test as no code change (just formatting)

### Are there any user-facing changes?

None
* GitHub Issue: apache#41173

Authored-by: Laurent Goujon <laurent@apache.org>
Signed-off-by: David Li <li.davidm96@gmail.com>
tolleybot pushed a commit to tmct/arrow that referenced this pull request May 4, 2024
…iles (apache#41174)

### What changes are included in this PR?

To make sure that all pom.xml files follow the same convention regarding indentation and organization, add spotless plugin with sortPom module to  check and format the files automatically.
    
Update Java developer documentation to provide the link to the  convention and instructions on how to use spotless
    
Finally apply the convention to all `pom.xml` files

### Are these changes tested?

No new test as no code change (just formatting)

### Are there any user-facing changes?

None
* GitHub Issue: apache#41173

Authored-by: Laurent Goujon <laurent@apache.org>
Signed-off-by: David Li <li.davidm96@gmail.com>
rok pushed a commit to tmct/arrow that referenced this pull request May 8, 2024
…iles (apache#41174)

### What changes are included in this PR?

To make sure that all pom.xml files follow the same convention regarding indentation and organization, add spotless plugin with sortPom module to  check and format the files automatically.
    
Update Java developer documentation to provide the link to the  convention and instructions on how to use spotless
    
Finally apply the convention to all `pom.xml` files

### Are these changes tested?

No new test as no code change (just formatting)

### Are there any user-facing changes?

None
* GitHub Issue: apache#41173

Authored-by: Laurent Goujon <laurent@apache.org>
Signed-off-by: David Li <li.davidm96@gmail.com>
rok pushed a commit to tmct/arrow that referenced this pull request May 8, 2024
…iles (apache#41174)

### What changes are included in this PR?

To make sure that all pom.xml files follow the same convention regarding indentation and organization, add spotless plugin with sortPom module to  check and format the files automatically.
    
Update Java developer documentation to provide the link to the  convention and instructions on how to use spotless
    
Finally apply the convention to all `pom.xml` files

### Are these changes tested?

No new test as no code change (just formatting)

### Are there any user-facing changes?

None
* GitHub Issue: apache#41173

Authored-by: Laurent Goujon <laurent@apache.org>
Signed-off-by: David Li <li.davidm96@gmail.com>
vibhatha pushed a commit to vibhatha/arrow that referenced this pull request May 25, 2024
…iles (apache#41174)

### What changes are included in this PR?

To make sure that all pom.xml files follow the same convention regarding indentation and organization, add spotless plugin with sortPom module to  check and format the files automatically.

Update Java developer documentation to provide the link to the  convention and instructions on how to use spotless

Finally apply the convention to all `pom.xml` files

### Are these changes tested?

No new test as no code change (just formatting)

### Are there any user-facing changes?

None
* GitHub Issue: apache#41173

Authored-by: Laurent Goujon <laurent@apache.org>
Signed-off-by: David Li <li.davidm96@gmail.com>
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.

3 participants