[Build] Make be easy code formatting

[tomoya0x00]

We've already checked the code format of PRs by Format GitHub Action. But the contributors may forget to format their own code before creating their PRs. It would be better to introduce a way to avoid forgetting it.

[tomoya0x00]

ref: the result of Format GitHub Action

https://github.com/DroidKaigi/conference-app-2023/pull/39

• Build GitHub Aciont is failed
• reviewdog suggests proper changes

[tomoya0x00]

How about introducing Spotless plugin for Gradle or/and suggesting installing Spotless Gradle Idea plugin in the README.md?

[tomoya0x00]

@takahirom Please let me know if what I am trying to do is out of line with your intentions.

[imsakuu]

But the contributors may forget to format their own code before creating their PRs. It would be better to introduce a way to avoid forgetting it.

I think that makes sense. For example, how about using pre-commit in Git hooks? In this case, we would need to have a script to set up the pre-commit and have the contributor run it themselves.

[tomoya0x00]

Thanks for your comment!

For example, how about using pre-commit in Git hooks?

Yeah, it is one of the solutions. I'm not familiar with Git hooks, so I didn't give it as an option.

I wonder is it possible to apply pre-commit automatically when a contributor clones this repository?

[imsakuu]

I don't know how to apply them automatically, but it is possible to provide a simple way.

step1: write our-pre-commit

./gradlew spotlessCheck
...

step2: write our-setup-script.sh

...
cp our-pre-commit .git/hooks/pre-commit

step3: clone and run the script (This is all the contributor has to do.)

$ git clone ...
$ our-setup-script.sh

So, we can introduce in README.md or CONTRIBUTING.md that the setup script needs to be run after git clone.

[jmatsu]

Only trusted projects can tweak gradlew to install git-hooks automatically. I guess it might sound hacky though.

By the way, a git-hook is powerful and I believe it will be a great help in fact but it works only if configured properly. And also, it can be skipped by adding --no-verify option and it may prevent commit and/or push without their notices. This means a git-hook are not a solution but just a help. A similar solution is inserting a code format action to a build task graph, although it can also be no-op if they do not execute a build task.

Anyway, I'm not against you and hook-based solutions. I'd like to say the hook-based approach may be insufficient.

Okay, let's go back to consider the point of what are acceptable code changes. AFAIK, the previous format workflow sounds slightly strict for me because it has alerted when the changes break code formatting rules anyway. However, we don't have to reject the changes immediately when all of broken code styles in the changes can be fixed by auto-corrections. Because, the changes will be acceptable by applying the fix somehow after reviews are approved but before merging.

So I'd love to suggest that we should do the following two things:

• Introduce git-hooks or insert a format task to build sequence to reduce contributors' workload
• Apply auto-corrections before checking the style format.

[takahirom]

I'm not particular about using KtLint; any other method is fine as well.

Regarding the hook, I don't think it would be great if the experience becomes waiting for 30 seconds every time you commit. If it can be done within 10 seconds even when the project gets somewhat larger(About the size of the repository at DroidKaigi in previous years), that might be acceptable. Personally, I think it might be better to use CI to format on pull requests if possible, but I'm not sure if it's possible to modify forked repositories.

[takahirom]

However, I have never developed while formatting in Hook properly, so for me, there is no problem to try it in Hook and change it if there is a problem.

[tomoya0x00]

I think it might be better to use CI to format on pull requests if possible, but I'm not sure if it's possible to modify forked repositories.

I have discussed this with ChatGPT(GPT-4) and it seems difficult.

By the way, is it too strict to make gradlew assemble fail if there is a code format violation in a PR? We’ve already set up the Spotless plugin for Gradle in our project, and gradlew build fails if there is a violation. Similarly, I was wondering if it would be a good(or bad?) idea to make gradlew assemble fail and suggest that contributors run gradlew spotlessApply.

[tomoya0x00]

I think the code formatting can be applied to our main branch after a PR is merged into our repository, but it is going to be a bit difficult to see the commit log for the main branch. Also, I'm a bit afraid of automatically changing the main branch.

[jmatsu]

By the way, is it too strict to make gradlew assemble fail if there is a code format violation in a PR? We’ve already set up the Spotless plugin for Gradle in our project, and gradlew build fails if there is a violation. Similarly, I was wondering if it would be a good(or bad?) idea to make gradlew assemble fail and suggest that contributors run gradlew spotlessApply.

A similar solution is inserting a code format action to a build task graph, although it can also be no-op if they do not execute a build task. from https://github.com/DroidKaigi/conference-app-2023/issues/40#issuecomment-1513572112

Same here though, I'm afraid contributors can avoid it and editing from GitHub web UI cannot follow it.

I think the code formatting can be applied to our main branch after a PR is merged into our repository, but it is going to be a bit difficult to see the commit log for the main branch. Also, I'm a bit afraid of automatically changing the main branch.

We can run a workflow after the review is approved before merging.

[takahirom]

If we use pull_request_target, maybe we can use the privileges of the forked repository, so we can commit over format?

[tomoya0x00]

Summary of what we discussed in our online meeting today:

We think it would be possible to detect when a PR is approved and automatically merge it into the main branch of our Repository after code formatting.

The important points are the following.

• No security issues or low risk is desirable
• No significant increase in commit time as the code base grows
• No Takahirom-san that comments "Please run ./spotlessKotlinApply" in every PR like a bot
• No warning on formatting results by Android Studio

First, let’s change our GitHub actions. If a PR has a code format violation, then comment “Please run ./gradlew spotlessKotlinApply”.

[takahirom]

For reference, I found compose-samples push the formatted commit to the pull request to format. 👀 https://github.com/android/compose-samples/blob/f87eca899d608be5d5f6542cf46e225d65dc84bb/.github/workflows/main.yml#L37