[Enhancement]: Add GitHub Actions to automatically format the Java Files according to Checkstyle

[SaptarshiSarkar12]

Related problem

This project has a code style defined in the checkstyle.xml file in .checkstyle directory. Checking whether the code in the Pull Requests or branches follows the project's code style and applying fixes accordingly is a tedious work.

Suggested solution

I want to use OpenRewrite's Maven Plugin to automatically format the Java files using the Checkstyle configuration. I would also like to create a GitHub Actions to automatically run the formatter and push the changes to the branches or the PRs.

Alternatives

No response

Additional context

I have implemented this feature in an Open-Source project - Drifty and the GitHub Action also pushes the formatted files to the branches or PRs as you can see below :point_down: If this issue seems good to the maintainers and can add some value to the project, then, I would like to work on this issue.

[Frawless]

Hello, thanks for the issue. From my POV this is not a ideal step that should be done automatically. We have checkstyle verification setup that provide necessary info to the contributor in case the code is wrong. I understand that this manual step of fixing checkstyle errors could be annoying, but it force the contributor to learn the code style we follow and after several PRs the contributor will be used to our code style which is not too specific from my POV.

Btw I was looking on the plugin and I wasn't able to properly make it working with very similar checkstyle configuration as we have (for imports and header style).

[SaptarshiSarkar12]

@Frawless I think that the contributors will not require to see the project's code style. The reason is that - they can use their own code style in their IDEs and push their changes, GitHub Actions will run the plugin to automatically format the file according to the checkstyle configuration. The plugin does some checks for import also. I have that plugin setup in my Open-Source project. It had once fixed the order of imports.

[SaptarshiSarkar12]

I haven't checked the headers but if it does not work, I can open a bug report / feature request in their GitHub repo. Once that issue gets fixed, we can do the format for headers also. @Frawless What do you think?

[Frawless]

There is a Strimzi Community Meeting this Thursday. I think it should be discussed with other maintainers as part of issue triage and then we can proceed with next steps. Will you be able to join?

[scholzj]

I think that having a make or mvn command that will do this for the user before they make the commit could be useful. But I agree with @Frawless that it seems wrong to have this done in some CI and push the commit to the PR. I certainly would not want that for my PRs. My general expectation is that:

• The code opened in the PR is ready for review and not waiting for some CI that modifies it
• The code is tested by the contributor before opening the PR

A workflow like that also creates a mess in the commits and makes it harder to review changes after the PR is opened as you might get unnecessary rebasing/merging because users did not pull the changes from the remote branch but also because when they address review comments, it will not be easy to see the changes done for that as they would be in multiple commits (one with the user-fixes and one with formatting changes) I guess.

I would be also a bit concerned about the automatic formatting - from my experience tools like this are often also creating badly readable code for example by blindly insisting on some line length and introducing unnecessary line breaks. But that is a more generic observation, not necessarily with the particular project you suggest to use.

[SaptarshiSarkar12]

@Frawless I would love to join the meeting. Unfortunately, I would be busy on Thursday. So, will it be possible for any maintainers to discuss this issue in the meeting?

@scholzj I have tried to format Java files as a pre-commit and even pre-add process, using git hooks. But, that does not work. One thing can be done - the GitHub Action will run once the Pull Request is marked as ready for review. Once the formatting is done (which usually takes around 3-4 minutes), maintainers can review the PRs. So, there will not be any mess in the commits also. OpenRewrite's plugin does not actually make the code unreadable and has no issues with line length (it follows the rules as written in the checkstyle configuration). By doing this, contributors' burden will get removed and a fast-paced development might be seen. @scholzj @Frawless What do you think?

[scholzj]

One thing can be done - the GitHub Action will run once the Pull Request is marked as ready for review. Once the formatting is done (which usually takes around 3-4 minutes), maintainers can review the PRs. So, there will not be any mess in the commits also.

But it is not really ready for review if the formatting is not fixed yet. It would also create a huge waste of CI resources as other CIs will be triggered immediately for the first commit and possibly fail.

[SaptarshiSarkar12]

@scholzj That is the only way to make sure that the CI runs the least number of times in the PR. Moreover, the CI will run only on event of a PR being marked as Ready For Review. So, I don't see any waste of resource.

[scholzj]

@scholzj That is the only way to make sure that the CI runs the least number of times in the PR. Moreover, the CI will run only on event of a PR being marked as Ready For Review. So, I don't see any waste of resource.

Our CIs run when the PR is opened or when a new commit is pushed into it. They do not care about Ready for review and that is intentional as you often want to run it for Draft PRs. They will also trigger by the first commit pushed by the user into the PR and not only for the second commit with your formatting changes.

[SaptarshiSarkar12]

@scholzj GitHub Actions provide the capability to specify when the CI should run. So, I can make a GitHub Actions workflow file in .github/workflows/ directory that will run only when the PR is marked as Ready For Review. Does that make sense?

[scholzj]

No, sorry, it does not.

• We likely do not want to run it when the Pr is ready for review as that does not make much sense.
• This is not the only CI we have. We have various other CIs used in Strimzi and those are what I'm talking about.

[SaptarshiSarkar12]

@scholzj Then, is there any other condition when you think running the CI will be beneficial to the PR? Why should we use other CI providers when PRs in GitHub are our concern? Is this repo hosted somewhere else also? Sorry, but I couldn't get the reason for not creating the CI in GitHub Actions.

[scholzj]

Maybe you can actually check some of our PRs to see how they work, what kind of integrations are triggered etc. Just because someone uses GitHub, it does not mean they use GitHub actions for everything. The reasons for that might be different - cost, resources, performance, quality, stability, supported platforms etc. There are also many 3rd party services people integrate with. Most of these seem to be triggered when PR is opened and when a new commit is pushed to the PR. So with your action that in some cases doubles the number of commits, you create quite a big waste. That can mean that CIs will be slow because of full capacity, it might mean wasted money because of increased cost or even simple things such as wasted energy and environmental impact.

[SaptarshiSarkar12]

@scholzj I understand why you are using other CI providers. But, I am talking about using GitHub Actions specifically for this issue of formatting. An ubuntu-runner will work for the formatter. Am I failing to explain you something? Please let me know.

[scholzj]

Maybe you are talking only about your GitHub action. But we are concerned about the overall picture that includes all the CIs we use, how the PRs are reviewed and tested etc.

[SaptarshiSarkar12]

@scholzj Okay. Got it. So, would this issue be closed?

[scholzj]

As already explained by @Frawless - issues are triaged in the community calls where it will be decided what to do with it => whether this is something that seems interesting or not.

[SaptarshiSarkar12]

Okay. So, let's wait for the decision of the maintainers in the community call :smile:.

[scholzj]

Discussed on the community call on 30.11.2023: Thanks for the offer. The idea of having a GitHub action doing this formatting does not sound useful to us and we are not really interested in it. If would want to contribute a make or Maven command to run locally to do the formatting, that would be completely fine for us. Thanks.

[SaptarshiSarkar12]

@scholzj Thank you for introducing this issue on behalf of me :smile:! If you want, I can set up the maven plugin which will format the files according to the checkstyle configuration. Should I do that?

[scholzj]

If you want, yes, you can add that. Basically, as a separate command similar to for example how Spotbugs is done => where you can for example run mvn spotbugs:check to check it.

[SaptarshiSarkar12]

Yeah, I'll do that 👍. @scholzj Can you please assign this issue to me? I'll start working on this soon 🙂.