Should coding standards be enforced by the continuous integration server?

Question

Should coding standards/style be enforced by the continuous integration server running static analysis tools (ex. PMD, StyleCop/FxCop) and failing the build if the standards are not followed? What types of rules should not be used to fail the build?

Answer 1

Coding Standards should be there for a reason... and non-compliance should be checked.

However, not all violations should be considered as errors - just as not all compilation violations are. Where you draw the line has to be a company and a tool decision.

For example, MISRA defines its rules as Required and Advisory... I would suggest that Advisories would allow the build to proceed.

Answer 2

It's not completely unheard of and you will know if it will work for you only by trying it out. There are some steps that you could be taken before that.

First the team should decide on the standards together. Then tools such as ReSharper should be used to tell developers if they are not adhering to standards. Doing peer reviews on every task could further help.

After those steps have been taken it could be considered to put coding standard checks to CI-server. However it should still be considered if it's wise to have build break for not adhering to coding standards. The risk is that you will have lot of broken builds which might dillute the meaning of broken build.

Instead of making the build break, you could run the tools and have them create reports. If coding standard violations seem to be increasing, you can get the team together and figure out why it's happening.

Answer 3

Should coding standards/style be enforced by the continuous integration server running static analysis tools (ex. PMD, StyleCop/FxCop) and failing the build if the standards are not followed?

Those continuous integration checks need to be very, very fast. Any significant delays will mean your programmers are going to commit and lose track of their thought process while waiting for the results. Make it longer and they will commit and get a cup of coffee or chat with their office mates about the latest bungled performance of some sports team. Those delays are highly counterproductive. Some things are best left to the nightly build or the code review.

What types of rules should not be used to fail the build?

The subjective ones, to start with. How do you enforce the "Code shall be self-documenting or well-commented" rule? The "no magic numbers" rule? Those are things best left to the code review.

Another category is violations to the rules that have already been granted a waiver. Given any sizable code base, there is inevitably going to be some chunk of code where violating the standard is exactly the right thing to do.

Answer 4

As part of a Software Quality Improvement Plan we recently coded a series of code sniffs to integrate into our build process.

We build a lot, being a PHP application there is no real compilation so the build is really a unit test / static analysis / runner, and we can afford to spend a few cycles on this.

We had some code quality issues, and some legacy code with lots of issues.

Starting on the basis that if it does not fail the commit it will be ignored we started confirming commits against our 'desired' coding standard, and failing commits with errors that did not meet the standard.

Maintenance ground to a halt, even the simplest fix to a legacy component required the developer to reformat huge quantities of source, and the build was broken more often than not. Needless to say we changed the errors to warnings, and now they are, ignored and 'mostly' pointless.

So I would say this (learned from hard experience).

Make sure that the standard of your code base is close enough to the standard that you enforce that you do not require dev's to reformat volumes of code, instantly. Or.. You are prepared for and expecting the increase in effort.

Being a small team with a huge delivery requirement, we couldn't afford to switch the team to a huge re-factor operation. Our coding standards are now mostly handled by manual review, and the legacy is being re-written as part of a continuous improvement plan.

When I said that the warnings are 'mostly' pointless, well we now use them for recording statistics that allow us to measure kpi's that should keep showing improvement.

When we enforce the code sniffs again, we will start light, and introduce a few sniffs at a time until we have the standard enforced.

Answer 5

It will depend on your ultimate goal and strategy.

Forcing all good mentioned standards in CI server might seem very lucrative. However, it might not be practical for the big (more than 6 developers) development team, if it is done on each commit to the server. Waiting your server to respond after commit should NOT be long delay. It may potentially cause some down-time.

However, It is totally legitimate to block a commit if code (the actually change-set) has dependency issues or does not compile. However, failing code because of code layout and some naming convention might be too sever and not vital restriction for the CI server commit rules.

But it is very likely to be helpful rule if applied during the evening build.

In addition, re-factoring tools can assist in implementing and learnig standards - like the Resharper or JustCode usage by developers.