CI: Add Auto-Format Workflow for Java files

[SaptarshiSarkar12]

• I have searched the issues of this repository and believe that this is not a duplicate.
• I have searched the release notes of this repository and believe that this is not a duplicate.

Describe the feature

This project has a code style available in the codestyle directory. Checking whether the code in the Pull Requests follows the project's code style and applying fixes accordingly is a tedious work. I want to automate this check and it will also fix any code that violates the code style. The changes will be pushed to the source branch of the PR automatically. This check can also be applied for any branch of this repository.
I would like to work on this issue.

[AlbumenJ]

One things you may need to care about: GitHub will not grant write access for workflows triggered in Pull Requests.

[SaptarshiSarkar12]

@AlbumenJ Can the maintainers enable that option from the settings page of this GitHub repo? Then, I can get pull-requests: write permission 😄.

[AlbumenJ]

@AlbumenJ Can the maintainers enable that option from the settings page of this GitHub repo? Then, I can get pull-requests: write permission 😄.

This is disabled by GitHub and we cannot change it :)

[SaptarshiSarkar12]

@AlbumenJ It is not disabled by GitHub. It can be enabled. I have a repo which has this Workflow permissions enabled like this 👇

The above settings can be found in this project specific settings page which will be https://github.com/apache/dubbo/settings/actions
If this repo (Apache/dubbo) has those permissions, I can easily build a workflow which can push changes to pull requests' source branch.

[AlbumenJ]

@AlbumenJ It is not disabled by GitHub. It can be enabled. I have a repo which has this Workflow permissions enabled like this 👇 The above settings can be found in this project specific settings page which will be https://github.com/apache/dubbo/settings/actions If this repo (Apache/dubbo) has those permissions, I can easily build a workflow which can push changes to pull requests' source branch.

I think we can ask Apache for help. One thing I need to make it clear that is if someone want to submit some harmful actions and such actions will be enabled by default which have the write permission. A possible way to trigger by comment which is submit by members and run by comment trigger.

[SaptarshiSarkar12]

@AlbumenJ Yeah, comments in pull request can be used to trigger the GH Action. If that looks good to you, I can work on this 😁.

[AlbumenJ]

@AlbumenJ Yeah, comments in pull request can be used to trigger the GH Action. If that looks good to you, I can work on this 😁.

Great! You can have a try and be aware that security is important. It is unacceptable for someone to change others' code without members approval.

[SaptarshiSarkar12]

Thank you @AlbumenJ!
Yeah, I will add a check in the CI to verify if the comment is from any maintainers. So, that will ensure that no format is done without member's approval.

[SaptarshiSarkar12]

@AlbumenJ Please assign me this issue. I'll start working soon.

[SaptarshiSarkar12]

@AlbumenJ I think that comments in PR is not an effective solution. We can have something like this 👇
The PR should not be a draft PR and can run on push.
I think without push events, it will be very difficult to make formatting and push those changes. Git push would fail.
@AlbumenJ What do you think?

[Koooooo-7]

Hi @SaptarshiSarkar12 , I think change contributors' PR directly is not a good way.
Add lint checks in CI and mention user need do the format in contribution guideline （as well as in PR template） is enough to defend bad smells.
And config a github pre-commit hook or put the spotless into mvn lifecyle could also be possible solution.

[SaptarshiSarkar12]

@Koooooo-7 Yeah, that can be done. @AlbumenJ What do you think?

[AlbumenJ]

However, can we make a repo only pre-commit hook automatically?

[Koooooo-7]

However, can we make a repo only pre-commit hook automatically?

Technically, yes.
But it may need some dirty thing to implement it since Java world seems no something like husky in node.

When contributors clone the project, the .git folder and the .git/hooks/<hooks>.sample are auto generated, all of those hook files are marked executable already.

A feasible way is using the mvn lifecycle plugin (such as exec-maven-plugin) to run a lite java file (e.g. java gitHooks.java) to modify the .git/hook/pre_commit.sample to .git/hook/pre_commit and append mvn commands into it, such as mvn spotless.

What the prerequisite we need is there must have Java env when the gitHooks.java launched (Normally, it should be).
It should work and we don't need consider about cross OS things as we don't create the hook directly, either do something like chmod +x.

---

Besides the assumptions above.
IMO, for now, we could just add lint check in CI (In action, run mvn spotless, check if contains no-staged files), let the guideline(PR template) stipulates how to do contribution and format.
The format automation solution could put on the table later until there has more neat pre-commit hooks solution.
BTW: IIRC, dubbo migrates to gradle is on the roadmap also, isn't it？

[SaptarshiSarkar12]

@AlbumenJ Koy (@Koooooo-7 ) said the correct methods to implement the one you are thinking.
@Koooooo-7 The maven exec plugin can be used. It is a good idea. I think implementing both the CI and the plugin will be best from two perspectives. The reviewers can see if any of the checks (especially formatter/linter) has failed or not via the CI - which is going to reduce the time spent on reviewing PRs. On the other hand, contributors can get their changes aligned to the code style of this project - which will help them.
The CI will also help to prevent the contributors from submitting unformatted code. A situation of this case can be 👇
The contributor has commented out the exec plugin and after making changes, uncomment it and pushes his/her code - which bypasses the plugin run. In that case, the CI will help the reviewers to check if the code needs any formatting/linting. If it requires, GitHub Action will post a comment in the PR with the steps that he needs to do.
@AlbumenJ @Koooooo-7 How's that idea 😁?

[AlbumenJ]

However, can we make a repo only pre-commit hook automatically?

Technically, yes. But it may need some dirty thing to implement it since Java world seems no something like husky in node.

When contributors clone the project, the .git folder and the .git/hooks/<hooks>.sample are auto generated, all of those hook files are marked executable already.

A feasible way is using the mvn lifecycle plugin (such as exec-maven-plugin) to run a lite java file (e.g. java gitHooks.java) to modify the .git/hook/pre_commit.sample to .git/hook/pre_commit and append mvn commands into it, such as mvn spotless.

What the prerequisite we need is there must have Java env when the gitHooks.java launched (Normally, it should be). It should work and we don't need consider about cross OS things as we don't create the hook directly, either do something like chmod +x.

Besides the assumptions above. IMO, for now, we could just add lint check in CI (In action, run mvn spotless, check if contains no-staged files), let the guideline(PR template) stipulates how to do contribution and format. The format automation solution could put on the table later until there has more neat pre-commit hooks solution. BTW: IIRC, dubbo migrates to gradle is on the roadmap also, isn't it？

@Koooooo-7 Do you mean that users should exec mvn xxx before commit? Or something else? Please feel free to explain what our contributors should be doing on their computers?

The contributor has commented out the exec plugin and after making changes, uncomment it and pushes his/her code - which bypasses the plugin run. In that case, the CI will help the reviewers to check if the code needs any formatting/linting. If it requires, GitHub Action will post a comment in the PR with the steps that he needs to do.

@SaptarshiSarkar12 Indeed. It's a good idea to post comments to remind contributors.

[Koooooo-7]

Hi @AlbumenJ , user only need run mvn clean install after they clone this repo as they usually do, that's all.
I made a sample repo git-hook-maven as a PoC which you could have a look.

Hi @SaptarshiSarkar12 , Although I raised this solution , it still looks little bit cumbersome to me tho.
I still suggest that we just do those things only, for now.

• Contribution guideline update about code format.
• PR template reminds contributors need do code format.
• CI actions add the formatting/linting check, if it were failed in format, showing the result and how to do format (in CI output) to contributors.

If dubbo move to gradle soon, I suppose we could do the whole things by gradle at that time, instead of maintaining a weird GitHook.java in project. If it is not on the schedule, and you all agree with taking the plugin solution now, I'm also fine with it.

[AlbumenJ]

Hi @SaptarshiSarkar12 , Although I raised this solution , it still looks little bit cumbersome to me tho. I still suggest that we just do those things only, for now.

• Contribution guideline update about code format.
• PR template reminds contributors need do code format.
• CI actions add the formatting/linting check, if it were failed in format, showing the result and how to do format (in CI output) to contributors.

These suggestions all make sense to me.

Hi @AlbumenJ , user only need run mvn clean install after they clone this repo as they usually do, that's all.
I made a sample repo git-hook-maven as a PoC which you could have a look.

Got it. And this is a little magic to contributors.

If dubbo move to gradle soon, I suppose we could do the whole things by gradle at that time, instead of maintaining a weird GitHook.java in project. If it is not on the schedule, and you all agree with taking the plugin solution now, I'm also fine with it.

Moving to gradle is a good idea. However, it is hard for us to translate all configurations from maven to gradle. BTW, we maintain a Maven plugin and I have no idea about how to rewrite it without affecting any users.

[Koooooo-7]

Got it. And this is a little magic to contributors.

Yea, because we wanna contributors don't become aware of it and hooks can also work automatically.
Emmmm, it seems quite tricky tho.

If dubbo move to gradle soon, I suppose we could do the whole things by gradle at that time, instead of maintaining a weird GitHook.java in project. If it is not on the schedule, and you all agree with taking the plugin solution now, I'm also fine with it.

Moving to gradle is a good idea. However, it is hard for us to translate all configurations from maven to gradle. BTW, we maintain a Maven plugin and I have no idea about how to rewrite it without affecting any users.

I see, maybe I have token the wrong memo that I had seen something about dubbo and gradle thing in the early of this year.

It sounds migrating to gradle hasn't put on the table yet. And it exactly a long terms thing, especially, the sort of compatibilities issues and avoid it impact too much for users.
TBH, I haven't been aware much of the Maven plugin of dubbo, so I may not have a valuable idea here.
If the plugin is for the dubbo build things, a new gradle one should be able to supplant it. Other functions stuff, it needs take time to find solutions.

BTW, personally, I think if some breaking change is inevitable, just let it be. if worthy. :)

[SaptarshiSarkar12]

@Koooooo-7 @AlbumenJ Thank you for the valuable suggestions.
So, I will make a CI pipeline to do these things in this order

• Check if the contributor's code changes align to the codestyle of this project
• If yes, well and good 👍. If not, post a comment providing the necessary details on how to fix those issues.
  @AlbumenJ @Koooooo-7 Can you please confirm once if the above things need to be done? Am I missing something out?