Proposal: Run cargo fmt before commits to improve PR workflow

[niStee]

Dear fellow contributors,

I'd like to propose a change to our project's development workflow that I believe will improve the efficiency and quality of our pull request (PR) process. Currently, we are checking for code formatting issues in the PR build, which can sometimes lead to failed builds and delays in the review process.

Instead, I suggest that we adopt a policy of running cargo fmt before each commit. This would ensure that the code is properly formatted before it is pushed to the repository, eliminating the need to check for formatting issues in the PR build.

The benefits of this approach include:

Faster PR Review: By having the code properly formatted before the PR is submitted, reviewers won't have to spend time checking and fixing formatting issues. This can significantly speed up the review process.

Consistent Codebase: Applying cargo fmt consistently across all commits will help maintain a clean and consistent codebase, which can make the project easier to maintain in the long run.

Reduced Build Failures: Eliminating formatting-related build failures will make the overall development workflow more reliable and predictable.

Improved Developer Experience: Developers won't have to worry about their commits failing due to formatting issues, which can be frustrating and time-consuming to fix.

I believe this change will have a positive impact on our project's development process, and I'd like to hear your thoughts and feedback on this proposal. If we all agree, we can update our contributing guidelines and start implementing this new workflow.

Thank you for your consideration.

Best regards,
Nils

[lucaspar]

What would it look like in practice? A pre-commit hook?

[SteveLauC]

Hi, thanks for the proposal!

First, I wanna say I am not against this.

But let's step back and take a look at the issue it tries to solve:

which can sometimes lead to failed builds and delays in the review process.

Unformatted code will lead to a failed build because other checks need the format check to succeed before being started, but IMHO, it won't delay the review process, if a contributor finds the format check failed, it is very simple to run cargo fmt --all-features locally and push the committed changes.

What would it look like in practice? A pre-commit hook?

Pre-commit hook is not compulsory, a developer can unlink the commit script and thus skip the check.

[niStee]

Please have a look at my suggestion for a possible solution in #957.

[SteveLauC]

Please have a look at my suggestion for a possible solution in #957.

IMHO, Let the CI change contributors' code may surprises them, I am not quite sure we should do this.