3

Our team has recently agreed on some very light coding standards, and that we need a means of enforcing them. We already have a mature Continuous Integration practice including frequent, small check-ins and pre-tested commits.

We're considering two methods for implementing an enforcement mechanism: a pre-commit SVN hook, or a dedicated build configuration on the CI server. Here's the trade-offs we've considered so far:

  • An SVN hook ...
    • (-) requires SVN server admin privileges to setup, which we don't have. We can work with IT, but avoiding the overhead is nice.
    • (-) points to a script, which we'd like to keep under version control with the rest of the product. I don't immediately know how to point at the correct hook-script, taking into account branches, tags, and trunk work.
    • (-) requires more work for reporting. The script would need to handle both generating and publishing the report.
    • (+) gives quick feedback.
    • (+/-) provides hard enforcement: the code checks-in, or it doesn't . Usually a good thing (it's called "enforcement" for a reason), but may give false positives.
  • The CI build configuration ...
    • (-) consumes time on both the CI server and an agent. Might be trivial, but does add up.
    • (-) introduces some extra latency, making feedback less immediate.
    • (+) easily handles standards that evolve over time, by pointing to the version of the script that exists in whichever branch we're working on.
    • (+) provides easier reporting. We already use the CI system for other reports. Adding a new one would be straight-forward.
    • (+) configurable enforcement: easier to soften the requirements, if necessary. We could collect a list of violations to fix out-of-band, or provide warnings about standards we're adopting on a trial basis.

What are the trade-offs of these different methods in enforcing coding standards here?

Related: Should coding standards be enforced by the CI server? - Raises some good points, but doesn't consider SVN hooks.

| improve this question | |
  • Silly question: Why not get the best of both worlds by using both? Restrict checkins via the svn hook(s) so the repo stays clean, and you get fast feedback and get the pretty pictures with CI reporting. – Steven Evers May 1 '14 at 18:57
  • @Steve Evers: We'd like to use a canonical standard for both, to avoid repeating the checks in code. See the point about taking into account branches, etc. – Lemur May 8 '14 at 23:19
3

You have missed one more option, execute the enforcement of coding standards at compile time (if you are working with a compiled language) or from your development environment (if you have one). That way the developers get immediate feedback regarding their violations of the coding standards. At first most developers will find it annoying, but they will soon get used to it (I did) and start coding in the right style, thereby not making violations of the coding standards anymore.

Obviously you'll also need to verify this somewhere else to make sure that nothing slips by. I would prefer to use the build server for this because

  • No necessary changes to the version control system, thus not slowing down commits etc.
  • Still have the ability to commit code even if it's not technically correct so it is possible to commit code to branches for safe keeping or experiments.
  • The CI build is used to notify developers all kinds of errors, an extra type of error is not too unexpected. Having the errors come from the commit however would seem to be unexpected(?).

Finally if you use the SVN hooks think about this, the SVN hooks might not allow you to check in code that violates the coding style but you can still commit code that is broken. That somehow seems backwards to me given that you should probably care more about code that doesn't compile or run than about code that violates the coding standards.

| improve this answer | |
  • Good point! Checking the standards at compile time is less about enforcement, as you point out, and more about early detection, with an opportunity to fix. Worth calling out, though. – Lemur May 8 '14 at 23:14
  • Checking the standards at compile time is more about education - providing a mechanism for the programmer to learn about (and comply with) the rules as they go along, rather than having to fix things later. – Andrew Jun 9 '15 at 13:28
2

The svn-commit hook approach will have problems scaling beyond anything more than 'no tabs/leading spaces in file', 'right type of new line characters' and perhaps 'indentation is not completely borked'. Otherwise you will:

  • upgrade your tool or configuration
  • want to make some trivial, urgent change to a file
  • be unable to do so without entirely rewriting it to clean up everything that is no longer current best practise.

As a result of which you will likely never be able to upgrade your coding standards. Which may be ok now, but will suck by 2020 or so...

Kept to that simple level, hooks are still pretty useful though. Keeping such problems entirely out of your history can simplify diffs.

| improve this answer | |
  • +1 for thinking about the effects of future upgrades. Thanks! – Lemur May 8 '14 at 23:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.