[DRILL-5184] Remove check style restriction on brackets for one-line statements

• Type: Improvement
• Status: Open
• Priority: Minor
• Resolution: Unresolved
• Affects Version/s: 1.8.0
• Assignee: Unassigned
• Reporter: Paul Rogers
• Created: 09/Jan/17 21:18
• Updated: 10/Jan/17 00:47

Description

Drill's build currently enforces a check style rule that all control statements must have brackets. For example:

if (true)
  doSomething();

is perfectly valid Java: the construct with well-known semantics inherited from C.

However, check style rejects the above. It must be:

if (true) {
  doSomething();
  }

Is this a good rule? Possibly. It may prevent the occasional bug. The real question is: is the benefit worth the cost? What is the cost? The cost is that developers tend to write legal Java code, do a build, wait for the build to fail, then must go back and fix code. The cost is thus x% of builds must be redone for trivial errors in check-style rules. In my own case, about half of builds fail because I'm in the habit of writing valid Java, not Drill-specific java...

So, the question is, is the savings derived from avoiding some hypothetical bug worth the very real cost of reducing developer productivity?

At the very least, make check-style issues into warnings to be ignored in development builds, cleaned up when preparing code for a PR.