Would it be a bad idea to periodically run code formatters on a repository?

Question

I'm thinking of creating a cron job that checks out code, runs code formatters on it, and if anything changed, commits the changes and pushes them back.

Most projects that use autoformatters put them in a git hook, but doing it automatically every few hours would remove the burden for each dev to install the git hook.

I would still encourage everyone to write clean, well-formatted code, and maybe I can have the system automatically ping devs when code they wrote gets reformatted, so they know what to do in the future.

Answer 1

Sounds nice, but I would prefer to have people responsible for committing code changes, not bots.

Besides, you want to make absolutely sure that those changes do not break anything. For example, we have a rule that orders properties and methods alphabetically. This can have an impact on functionality, for example with the order of data and methods in WSDL files of WCF contracts.

Answer 2

I would instead try to make it really easy for everyone in the team to apply automatic code formatting according to your team's standard directly inside your editor or IDE, to the current source code file (or selected parts of it). This gives your team members more control about how and when the formatting takes place, let them inspect the code before it is committed in the final form, and test it after the formatting took place, not before.

If all or most of your team members use the same editor, this should not be too hard. If everyone uses a different one, then your approach might be the second best solution, as long as the team supports this. However, I recommend you have extra safety measures installed, like nightly builds and automated tests which are run after each automatic code modification.

Answer 3

It's a bad idea, not just because it discourages people from writing decent code, but also because the reformatting will show up as code changes in your VCS (you do use one I hope), masking the historical flow of the code's development. Even worse, every code formatting action (indeed every change to the code at all) has the possibility to introduce bugs, whether it's manual or automated. So your formatter can now introduce bugs in your code that aren't going to go through code reviews, unit testing, integration testing, etc. until possibly months later.

Answer 4

I would tend to believe it is a good idea (to automatically run code formatters), but that is just my opinion.

I won't run them periodically, but if possible before version control commits.

With git, a pre-commit hook doing that would be useful. In many C or C++ projects built with some Makefile, I am adding some indent target (which run suitably code formatters like indent or astyle) and expect contributors to run make indent regularly. BTW, you can even add some make rules to ensure that the git hooks have been installed (or to install them).

But really, it is more a social issue than a technical one. You want your team to commit clean and nicely formatted code, and that is a social rule of your project. (There is not always a technical answer to every social problem).

Version control is mostly some tool to help communication between human developers (including your own self a few months from now). Your software don't need VC or formatting, but your team does.

BTW, different communities and different programming languages have different views on code formatting. For example, Go has only one code formatting style, but C or C++ have a lot of them.

Answer 5

I think it's a bad idea. Many of the answers already covered that it dirties the history by making it difficult to pin down who actually added a line and that it encourages people to just commit whatever and the format-bot will handle it.

A better approach would be to incorporate a format checker into your build tool. (In Java there is Checkstyle) Then, only allow people to merge their branches to the main branch if the build passes (including the formatting).

If you allow people to commit straight to the main branch (like in Subversion for example) then you will still need to make sure everyone has the discipline to commit only formatted code (or have the server only accept the commits once some checks have been run).

Answer 6

In general, I think it is a bad idea. In principle it's a valid idea, but it can be problematic in reality. Having the code formatter break your code is a real possibility, and it only takes one formatting run to have your developers respond with (probably justified) hostility (e.g. "Your lousy code formatter broke the build, turn it off now!").

In the same vein as @BasileStarynkevitch's recommendation, we use git server-side post-receive hooks to send "advisory emails" about code style.

If I push a commit that contains style violations, the git origin server will send me an email that informs me that I broke the style guidelines and recommends that I fix my code. It does not, however, enforce this because there may be valid reasons for breaking the house style (long strings exceeding line length limit, for example).

If it's a systemic problem that is harming the codebase, it might be time to start bringing up code style issues in code reviews. Poor code style may mask bugs and make the code harder to read, so it can be a valid code review issue.

To add to the "social problem" aspect of things, it can be worthwhile encouraging people to fix cosmetic and stylistic defects as they find them. We have a standard commit message "Cosmetic." for code style fixes that other developers know do not contain breaking changes.

As @DocBrown says, another option is to enforce code style within your IDE. We use CodeMaid with Visual Studio to correct many common style errors. It will run upon saving the code files, meaning that bad-style code should never even make it into the repo... in theory :-).

Answer 7

Yes, I think it's a bad idea. Don't get me wrong, the reason to do it sounds great, but the result could still be horrible.

You will have merge conflicts when pulling a tracked branch, atleast I fear that would be the case, I might be wrong though.

I don't want to test it right now at work, but you should try it out yourself.

In fact you can just check out a recent commit. Make a new branch, commit something petty, cherry pick or merge with no autocommit.

Then run your script, pull and if your result is a horrible merge mess, then you should definitly not do this, at daylight.

Instead you could potentially put it into a nightly build or a weekly build.

But even a nightly might be a bad idea.

You could either run it weekly, when you are sure no merge conflicts will arise because everything is finished on monday.

Otherwise run it 1-2 times a year on holiday season, when merge conflicts will not occur.

But the solution might depend on your priority for code style.

I think that making a setup script that automatically creates the git repository and sets the hooks for the project would be better.

Or you might include the hook setup script in a folder for your devs within the project and simply check it into git itself.

Answer 8

Something I haven't seen mentioned is the fact that sometimes there are legitimate reasons to not format something according to a set of rules. Sometimes the clarity of the code is improved by going against a given guideline that 99% of the time makes sense. Humans need to make that call. In those cases, automatic code formatting would wind up making things less readable.

Answer 9

It’s an awful idea.

If one of my developer colleagues made gratuitous changes to source files, it wouldn’t pass a code review. It just makes life harder for everyone. Change the code that needs changing, nothing else. Pointless changes lead to merge conflicts, which can lead to errors, and just create pointless work.

If you want to do this on a regular basis, that’s just awful.

And then there is the question of what kind of changes the code formatter makes. I use automatic formatting in my editor, it works reasonably well, and I can improve things when the automatic formatting is less than perfect. If you use a code formatter that goes beyond that, you are not going to improve the code, you are going to make it worse.

And then there is the social problem. There are people who want to force everyone to use their code style, and there are more flexible people. Something like this would likely be suggested by the "grammer-nazi" (spelling intentional) type of developer who want to force their style on everyone else. Expect a backlash, and expect the flexible, normally easy-going developers to put their foot down.

Answer 10

You don't mention the VCS you use but depending on that another option is to have a server-side hook. A VCS like git supports that. You could install a sever-side hook that runs the formatter on the version being pushed and then compares the formatted file against the version that is being pushed. If they differ the developer did not use the correct formatting and the server could reject the push. This would force your devs to only push code with the desired formatting thus encouraging them to write clean code from the beginning, it would make the devs responsible for having tested the correctly formatted code and relieve your devs from having to manually install a client-side hook.

Answer 11

It's a trade-off between a cleaner code format and more exact and easier to understand git history. Depends on nature of your project and how often do you dive in git history or blame to understand what's going on. If you're working on something new and don't have to maintain backwards compatibility, history usually doesn't play as important role.

Answer 12

This idea is similar to some other answers, but I cannot comment my suggestions.

One option is to set an alias (or hook, or whatever) to the commit function, that runs the code formatter on the to-be-committed code before it is committed.

It could have 2 (or more) results:

1) Show the user the proposed changes and ask for their approval to apply and commit the changes.

2) Ignore the proposed changes and commit the original code.

You could also add more flexibility to these options, like the ability to edit the changes proposed in option 1. Another idea (Depending on how hard you want to push these coding standards) is to have the system send you a report of some sort when option 2 is selected.

This may be a good balance of auto-checking all code like you want, while still allowing flexibility to the developers to suit their needs. It also allows the option to not 'auto reject' code with formatting differences as mentioned in other answers. With the 'I have reviewed and approve automatic formatting corrections; commit' option, it still keeps personal accountability for each developers work and doesn't mess with VCS.

Answer 13

I won't do it on the repository but I will do it on save if the tool supports it. Eclipse is one and in addition I would do the code cleanup including sorting.

The nice part is that it is part of the project so every developer will get it for their project.

As an added bonus merges would be significantly simplified since things will not be jumping around.

Code reviews will prevent any errant ones.

Another place I would do it is have it part of the build. In my case I have it such that maven builds will reformat the XML and clean up pom files and reformat the code.

That way when developers are ready to push it will all be cleaned up for their pull request.