✨ Unify code quality improvements

[aaronmondal]

Many code-related PRs to this repo address formatting and linting issues.

Without a commonly agreed-upon set of tools PRs with code quality fixes might override each other. This is of course not ideal. A unified approach to code style would be nice.

I think we can fairly easily enforce something like this by e.g. successively adding corresponding tools to #441 in case that gets merged.

Languages:

At the moment everything below here is a suggestion. Please share any insights on better options.

Scala

Formatter: Scalafmt seems like a compelling option as it has editor integration for virtually every IDE and is also available via many package managers, including nixpkgs. Linter: Does Twitter have guidelines somewhere?

Java

Does Twitter have guidelines somewhere?

Starlark

Formatter: Should be buildifer in formatting mode. Linter: Should be buildifier in linting mode.

Python

Literally every non-AI python project is a linter of formatter, so this is a hard choice. Twitter has an archived guide for python for now, but it is unclear to me how we can integrate this in a pre-commit hook. It might be possible to configure yapf for formatting, but then linting is unclear. A personal favorite of mine is wemake-python-styleguide which essentially aggregates half of the flake8 universe and obsoletes formatters entirely.

C++

Formatter: Should be clang-format. A simple .clang-format like

---
BasedOnStyle: Google

works wonders for a C++ codebase.

Linter: Should be clang-tidy. Getting code truly conformant with strict rules can be very hard though. I'd suggest starting with something basic here and then gradually increasing strictness. A no-op config looks like this:

---
Checks: |
  -*

WarningsAsErrors: "*"

This can then gradually be improved to e.g. a config like this.

Rust

Formatter: rustfmt? Seems like the default rustfmt config makes most sense. Linter: clippy? May be run in a relaxed configuration in the beginning and then gradually made stricter.

Suggestions for CQA PRs

My personal recommendations:

• Clearly separate formatting PRs, linting PRs and manual changes.
• A formatting PR needs to be reproducible. Include the formatting command in the PR description so that reviewers can verify the diff.
• If a formatting PR gets too large to review, consider splitting it into several smaller ones, but add the already formatted files to CI to avoid regressions. Tools like sapling might be helpful here.
• A linting PR should only flip a single warning or tool, but fix it globally and add it to the CI configuration to avoid regressions by future commits.
• If a linting PR is really small, it might be viable to group multiple fixes in a single PR

[aaronmondal]

Related:

• 441

• 952

• 922

[Foxlass]

Rewrite in Rust

[ajskateboarder]

Scala

Does Twitter have guidelines somewhere?

This could just be scalafmt

[aaronmondal]

This could just be scalafmt

Interesting! It's also available as nix package, so should be fairly easy to integrate. I've updated the OP.