Domanda

This question comes from previous experiences with svn.

We had a problem where our code was badly formatted, mainly because of the increase in developers at the time who did not adhere to our formatting guide for various reasons. So there was discrepancies in code formatting for quite a bit of time. Additionally the codebase was quite old as well, so it was probably going on for years.

Anyway, there was a suggestion to format all touched classes in new features into our agreed standard code formatting style. Other developers were against this mainly because:

  • You can't use an annotation style view of the code, so you can't see what features a particular line of code is related to.

In the end, we ended up rejecting this idea and only did formatting for newly introduced class files. For new code in existing classes, we only ensured that the lines that were edited were indented correctly.

We are mostly eclipse users, but I think that this feature is available in other ide's as well (just checked online for netbeans and intellij).

For an additional bit of context, the codebase in question was fairly mature and there was a fair balance between support work and new feature work. Sometimes new features were split into phases, so this is also something to consider.

So the question(s) are:

  • Was this the correct approach? Is it reasonable to reject "code formatting commits" just to preserve an annotated view of the code?
  • Are there better ways of doing this? My opinion here is that maybe there should have been a commit hook that does automatic formatting, not sure if something like this is exists or is even valid though?
È stato utile?

Soluzione

You do not want to do this in a hook. Not only is it a really bad plan to have a hook edit the source in any way as it's in transit, but the error the user will get is going to suck (it would be impossible to tell the user all of the errors in a meaningful way on an error message). I'm not a fan of enforcing style in general.. but if you must, I recommend a three part plan

  1. Define and document your coding style. What you come up with isn't as important as the discussion.
  2. Export configs for editor and share with team. this will be editor dependent. you want to make it stupid easy so that the drudgery of indentation and other syntax things is handled automatically.
  3. Have your CI enforce that style. If you really want to enforce it, you'll do it here. likely with something like checkstyle. This will generate a fancy report that will show and display all the errors.

I've worked on teams that did this really well... the standard was pretty flexible, everyone felt they had input so when the build failed there was no grumbling. We fixed it and moved on.

I did work on a team where an irrational fascist believed that the style was more important than the completeness or correctness of the code. It sucked.

Personally I'd stop at step two and see where that gets you before moving on to step 3.

Altri suggerimenti

In Eclipse, under Preferences > Save Actions, one can enable the Auto Formatter which formats the code on file save. You can configure a set of Formatting Rules and Export Preferences File to send to your developers (or have the leading developer do it) for them to Import Preferences File, so the code is automatically formatted as they are developing it.

A standardized auto-formatter would help eliminating the in-similarities in code style which causes non-codebase changes in the SVN.

I think it would be perfectly fine to do a one off code formatting commit for the old classes in your repo, it's definitelly better then having ill-formatted code all around the place.

As for IDE's Intellij and Eclipse can do formatting on save, but I'd be against doing it in a hook because it's not aware about what you're coding in, and wouldn't trust that on a script. But that can work as well but need to make sure that what you're using for formatting is well up to the task

Autorizzato sotto: CC-BY-SA insieme a attribuzione
Non affiliato a StackOverflow
scroll top