[blauditore]

About 4. Style over substance:

Coding style (apart from formatting) can be crucial for understandability, so it's important to have a critical look at it when reviewing. For instance, consider the following snippets:

  void myMethod(FooBar arg) {
    if (arg != null) {
      arg.foo(something);
      for (int i = 0; i < arg.bars().size(); i++) {
        arg.bars().get(i).baz();
      }
    }
  }

vs

  void myMethod(FooBar arg) {
    if (arg == null)
      return;

    arg.foo(something);
    arg.bars().forEach(baz);
  }

Even though they're functionally identical, they look wildly different. These things can hardly be judged by an automatic process like linting, so it's important to manually do that.

[rocky1138]

For me, a lack of adherence to our conventions is a smell that not all i's have been dotted and t's have been crossed. If the style is poor, I start to think the logic may be suspect, as well.

This is doubly true when delinting and styles are automatic via git hooks.

[glebm]

> These things can hardly be judged by an automatic process like linting

Both of these (if vs guard clause, loop vs each) are enforced by the Rubocop linter in Ruby.

[throwawayish]

Today in: Broadly missing a point.

It's good and nice that there is one linter for a particular language that catches 100 % of these issues in an example, but that doesn't do much to change the fact that most linters for most languages are unable to do this, and that there are quite some instances of these decisions where it is doubtful that any linter would be able to lint it.

[glebm]

> most linters for most languages are unable to do this,

In the long run improving the linter is better for everyone. The examples given by the OP as infeasible are feasible.

Focus on the design over trivial style issues. Your style guide should be 95% covered by the linter. If it isn't, fix your linter.

> Broadly missing a point

My points is: Underestimating static analysis will lead you to the wrong conclusions.

[glebm]

> some instances of these decisions where it is doubtful that any linter would be able to lint it.

Such issues are rarely style issues. They are more in the design territory where the focus of code reviews should be.

[scottnonnenberg]

Oh, I certainly agree that coding standards are important. Consistency of coding style makes a codebase substantially more approachable. I'm just interested in making that as automated as possible: https://blog.scottnonnenberg.com/eslint-part-3-analysis/