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

SOLR-17473: Introduce build tools formatting #2739

Merged
merged 6 commits into from
Oct 5, 2024

Conversation

malliaridis
Copy link
Contributor

https://issues.apache.org/jira/browse/SOLR-17473

Description

With the build tools persisting of various Java files, formatting and checking tools should also be run on these files.

Solution

This PR migrates the buildSrc to composite build project, as demonstrated in apache/lucene#13484 and applied in #2706 (with minor adjustments).

Tests

No tests have been added or altered.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide

Migrate buildSrc into a composite included build (build-infra).

Expose a plugin with buildinfra extension.
@malliaridis malliaridis force-pushed the SOLR-17473/build-tools-formatting branch from b545050 to 97a8c99 Compare October 2, 2024 14:07
Copy link
Contributor

@epugh epugh left a comment

Choose a reason for hiding this comment

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

I had no idea we had so many .java files related to doing our builds. I am always in favour of having more code strictness in an environment where many cooks are in the kitchen!

@epugh
Copy link
Contributor

epugh commented Oct 2, 2024

^ @malliaridis I triggered the github build and looks like something burped.

Copy link
Contributor

@dsmiley dsmiley left a comment

Choose a reason for hiding this comment

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

Since some of the build infra source files come from Lucene in their entirety, why not leave them be in the Lucene namespace? Lucene & Solr continue to have a build that's rather aligned, allowing taking of changes from one project to the other.

@malliaridis
Copy link
Contributor Author

@dsmiley I thought the naming was just a legacy thing in this case, but for just this case I made a separate commit for the renaming, so that I can undo it easily.

@epugh thanks for executing the pipeline. The error seems not related to the changes though. Any idea what could be the cause?

Copy link
Contributor

@dsmiley dsmiley left a comment

Choose a reason for hiding this comment

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

I did another short look in a hurry. If I had more time, I might compare to Lucene project but I don't. +1 approval from me. I'll merge tomorrow but maybe sooner with someone else's +1.

@dsmiley dsmiley merged commit f0b5291 into apache:main Oct 5, 2024
3 checks passed
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.

4 participants