chore: update docs for 1.0 (#190)

* chore: update docs for 1.0

* chore: fix in-flight collision making main red

Author: alexeagle

README.md

@@ -9,20 +9,11 @@ Features:
   Instead, users simply lint their existing `*_library` targets.
 - Lint checks and fixes are run as normal Bazel actions, which means they support Remote Execution and the outputs are stored in the Remote Cache.
 - Lint results can be **presented in various ways**, such as Code Review comments or failing tests.
-  See [Usage](https://github.com/aspect-build/rules_lint/blob/main/docs/linting.md#usage) below.
+  See [Usage](https://github.com/aspect-build/rules_lint/blob/main/docs/linting.md#usage).
 - **Can format files not known to Bazel**. Formatting just runs directly on the file tree.
   No need to create `sh_library` targets for your shell scripts, for example.
 - Honors the same configuration files you use for these tools outside Bazel (e.g. in the editor)
 
-This project is inspired by the design for [Tricorder].
-This is how Googlers get their static analysis results in code review (Critique).
-https://github.com/google/shipshape is an old, abandoned attempt to open-source Tricorder.
-It is also inspired by <https://github.com/github/super-linter>.
-
-[aspect cli]: https://docs.aspect.build/v/cli
-[tricorder]: https://static.googleusercontent.com/media/research.google.com/en/pubs/archive/43322.pdf
-[reviewdog]: https://github.com/reviewdog/reviewdog
-
 ## Supported tools
 
 | Language               | Formatter             | Linter(s)        |
@@ -82,9 +73,14 @@ Thanks!!
 
 > We'll add documentation on adding formatters as well.
 
-## Design
+## Installation
 
-Formatting and Linting work a bit differently.
+Follow instructions from the release you wish to use:
+<https://github.com/aspect-build/rules_lint/releases>
+
+## Usage
+
+Formatting and Linting are inherently different, which leads to differences in how they are used in rules_lint.
 
 | Formatter                                                         | Linter                                                 |
 | ----------------------------------------------------------------- | ------------------------------------------------------ |
@@ -96,49 +92,25 @@ Formatting and Linting work a bit differently.
 | Can always format just changed files / regions                    | New violations might be introduced in unchanged files. |
 | Fast enough to put in a pre-commit workflow.                      | Some are slow.                                         |
 
-This leads to some minor differences in how they are used in rules_lint.
-
-We treat type-checkers as a build tool, not as a linter. This is for a few reasons:
-
-- They are commonly distributed along with compilers.
-  In compiled languages like Java, types are required in order for the compiler to emit executable bytecode at all.
-  In interpreted languages they're still often linked, e.g. TypeScript does both "compiling" to JavaScript and also type-checking.
-  This suggests that rules for a language should include the type-checker,
-  e.g. we expect Sorbet to be integrated with rules_ruby.
-- We think most developers want "error" semantics for type-checks:
-  the whole repository should successfully type-check or you cannot commit the change.
-  rules_lint is optimized for "warning" semantics, where we produce report files and it's up to the
-  Dev Infra team how to present those, for example only on changed files.
-- You can only type-check a library if its dependencies were checkable, which means short-circuiting
-  execution. rules_lint currently runs linters on every node in the dependency graph, including any
-  whose dependencies have lint warnings.
-
-## Installation
-
-Follow instructions from the release you wish to use:
-<https://github.com/aspect-build/rules_lint/releases>
-
-## Usage
-
 ### Format
 
 To format files, run the target you create when you install rules_lint.
 
-We recommend using a Git pre-commit hook to format changed files, by running `bazel run //:format [changed file ...]`.
+We recommend using a Git pre-commit hook to format changed files, and [Aspect Workflows] to provide the check on CI.
 
 [![asciicast](https://asciinema.org/a/vGTpzD0obvhILEcSxYAVrlpqT.svg)](https://asciinema.org/a/vGTpzD0obvhILEcSxYAVrlpqT)
 
-See [Formatting](./docs/formatting.md) for more ways to use the formatter, such as a pre-commit hook or a CI check.
+See [Formatting](./docs/formatting.md) for more ways to use the formatter.
 
 ### Lint
 
-To lint code, we recommend using the Aspect CLI to get the missing `lint` command.
+To lint code, we recommend using the [Aspect CLI] to get the missing `lint` command, and [Aspect Workflows] to provide first-class support for "linters as code reviewers".
 
 For example, running `bazel lint //src:all` prints lint warnings to the terminal for all targets in the `//src` package:
 
 [![asciicast](https://asciinema.org/a/xQWU1Wc1JINOubeguDDQbBqcq.svg)](https://asciinema.org/a/xQWU1Wc1JINOubeguDDQbBqcq)
 
-See [Linting](./docs/linting.md) for more ways to use the linter, such as running as a test target, or presenting results as code review comments.
+See [Linting](./docs/linting.md) for more ways to use the linter.
 
 ### Ignoring files
 
@@ -162,3 +134,25 @@ But we're not trying to stop anyone, either!
 
 You could probably configure the editor to always run the same Bazel command, any time a file is changed.
 Instructions to do this are out-of-scope for this repo, particularly since they have to be formulated and updated for so many editors.
+
+## FAQ
+
+### What about type-checking?
+
+We consider type-checkers as a build tool, not as a linter. This is for a few reasons:
+
+- They are commonly distributed along with compilers.
+  In compiled languages like Java, types are required in order for the compiler to emit executable bytecode at all.
+  In interpreted languages they're still often linked, e.g. TypeScript does both "compiling" to JavaScript and also type-checking.
+  This suggests that rules for a language should include the type-checker,
+  e.g. we expect Sorbet to be integrated with rules_ruby and mypy/pyright to be integrated with rules_python or Aspect's rules_py.
+- We think most developers want "build error" semantics for type-checks:
+  the whole repository should successfully type-check or you cannot commit the change.
+  rules_lint is optimized for "warning" semantics, where we produce report files and it's up to the
+  Dev Infra team how to present those, for example only on changed files.
+- You can only type-check a library if its dependencies were checkable, which means short-circuiting
+  execution. rules_lint currently runs linters on every node in the dependency graph, including any
+  whose dependencies have lint warnings.
+
+[aspect workflows]: https://docs.aspect.build/workflows
+[aspect cli]: https://docs.aspect.build/cli

docs/formatting.md

@@ -88,7 +88,10 @@ $ chmod u+x .git/hooks/pre-commit
 
 ### Check that files are already formatted
 
-There are two ways.
+We recommend using [Aspect Workflows] to hook up the CI check to notify developers of formatting changes,
+and offer to apply them as a suggestion in the code review thread.
+
+To set this up manually, there are two supported methods:
 
 #### 1: `run` target
 
@@ -138,3 +141,5 @@ format_test(
 Then run `bazel test //tools/format/...` to check that all files are formatted.
 
 [Gazelle]: https://github.com/bazelbuild/bazel-gazelle
+[Aspect Workflows]: https://docs.aspect.build/workflows
+[Aspect CLI]: https://docs.aspect.build/cli

docs/linting.md

@@ -18,16 +18,32 @@ Finally, register those linter aspects in the lint runner. See details below.
 
 ## Usage
 
-### 1. (Coming Soon) Linter as Code Review Bot
+### 1. Linter as Code Review Bot
 
-An upcoming release of [Aspect Workflows](https://docs.aspect.build/workflows/) will include a `lint` task, which wires the reports from bazel-out directly into your code review.
+[Aspect Workflows] includes a `lint` task, which wires the reports from bazel-out directly into your code review.
 The fixes produced by the tool are shown as suggested edits, so you can just accept without a context-switch back to your development machine.
 
 ![lint suggestions in code review](./lint_workflow.png)
 
+We recommend this workflow for several reasons:
+
+1. Forcing engineers to fix lint violations on code they're still iterating is a waste of time.
+   Code review is the right time to consider whether the code changes meet your teams quality bar.
+2. Code review has at least two parties, which means that comments aren't simply ignored.
+   The reviewer has to agree with the author whether a suggested lint violation should be corrected.
+   Compare this with the usual complaint "developers just ignore warnings" - that's because the warnings were presented in their local build.
+3. Adding a new linter (or adjusting its configuration to be stricter) doesn't require that you fix or suppress all existing warnings in the repository.
+   This makes it more feasible for an enthusiastic engineer to setup a linter, without having the burden of "making it green" on all existing code.
+4. Linters shouldn't be required to have zero false-positives. Some lint checks are quite valuable when they detect a problem, but cannot always avoid overdetection.
+   Since code review comments are always subject to human review, it's the right time to evaluate the suggestions and ignore those which don't make sense.
+5. This is how Google does it, in the [Tricorder] tool that's integrated into code review (Critique) to present static analysis results.
+   With [Aspect Workflows] we've provided a similar experience.
+
+[Tricorder]: https://static.googleusercontent.com/media/research.google.com/en/pubs/archive/43322.pdf
+
 ### 2. Warnings in the terminal with `bazel lint`
 
-Aspect CLI adds the missing 'lint' command, so users just type `bazel lint //path/to:targets`.
+[Aspect CLI] adds the missing 'lint' command, so users just type `bazel lint //path/to:targets`.
 
 Reports are then written to the terminal.
 
@@ -45,9 +61,9 @@ lint:
 
 ### 3. Warnings in the terminal with a wrapper
 
-If you don't use Aspect CLI, you can use vanilla Bazel with some wrapper like a shell script that runs the linter aspects over the requested targets.
+If you don't use [Aspect CLI], you can use vanilla Bazel with some wrapper like a shell script that runs the linter aspects over the requested targets.
 
-See the `example/lint.sh` file as an example.
+See the `example/lint.sh` file as an example, and pay attention to the comments at the top about fitting it into your repo.
 
 [![asciicast](https://asciinema.org/a/gUUuQTCGIu85YMl6zz2GJIgD8.svg)](https://asciinema.org/a/gUUuQTCGIu85YMl6zz2GJIgD8)
 
@@ -59,7 +75,7 @@ This is the same flag many linters support.
 
 ### 4. Errors during `bazel build`
 
-Add `--@aspect_rules_lint//lint:fail_on_violation` to the command-line,
+Add `--@aspect_rules_lint//lint:fail_on_violation` to the command-line or to your `.bazelrc` file
 to cause all linter aspects to honor the exit code of the lint tool.
 
 This makes the build fail when any lint violations are present.
@@ -80,3 +96,6 @@ To bypass this filter, add `tags=["lint-genfiles"]` to a target to force all the
 
 Some linters honor the debug flag in this repo. To enable it, add a Bazel flag:
 `--@aspect_rules_lint//lint:debug`
+
+[Aspect Workflows]: https://docs.aspect.build/workflows
+[Aspect CLI]: https://docs.aspect.build/cli

example/tools/format/BUILD.bazel

@@ -88,6 +88,5 @@ format_test(
     starlark = "@buildifier_prebuilt//:buildifier",
     # FIXME: not hermetic: error while loading shared libraries: libFoundation.so
     # swift = ":swiftformat",
-    terraform = ":terraform",
     workspace = "//:.shellcheckrc",  # A file in the workspace root, where the no_sandbox mode will run the formatter
 )