Avoid code linting comments in review, if the linter doesn’t catch it.
This issue is a follow-up of our retrospective for %11.4.
As we (GitLab) want to ship more and increase our throughput, we (the Secure Team) found very annoying and time-wasting to deal with tiny code linting issues.
In Go, this problem doesn't exist (much), because the linter (go fmt, go vet, ...) reformats the code in a reproducible way.
In ruby, we can have many ways of expressing the same functionality, and even the same code. Each maintainer and developer will have her own habits and desires in that area. Now we can argue that some formats are common in the GitLab codebase, and must be enforced.
We agreed during our retrospective that if something must be enforced company-wide, then it's the job of Rubucop (or any other linter, as soon as it's automated). If there's no rule in the linter, than the reviewer should either:
- Create a follow-up issue to update the linter
- Leave the code as it Otherwise, we can spend a lot of time doing and undoing. It's almost impossible for developers to anticipate the mind of the reviewer, and going back and forth on these details are making them losing focus on the feature itself.
This issue should lead to an addition to this handbook, so the process would prevent from creating these linting comments in the future.
/cc @dhavens @gl-secure