From 07d83f656942d49364d463a2a675dcbd40a5ea77 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Thu, 9 Nov 2023 15:45:07 -0800 Subject: [PATCH] style: recommend format as an alias in root BUILD (#51) * style: recommend format as an alias in root BUILD * docs: code review comment * docs: code review comment * fix: correct targets that moved to tools package --- README.md | 2 +- docs/formatting.md | 21 +++++++++++++++------ example/BUILD.bazel | 34 ++++++++++++++-------------------- example/tools/BUILD | 22 ++++++++++++++++++++++ example/tools/lint.bzl | 8 ++++---- 5 files changed, 56 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 23690b15..c915e6f8 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,7 @@ Follow instructions from the release you wish to use: 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 //tools:format [changed file ...]`. +We recommend using a Git pre-commit hook to format changed files, by running `bazel run //:format [changed file ...]`. [![asciicast](https://asciinema.org/a/vGTpzD0obvhILEcSxYAVrlpqT.svg)](https://asciinema.org/a/vGTpzD0obvhILEcSxYAVrlpqT) diff --git a/docs/formatting.md b/docs/formatting.md index 64c3b572..953167f7 100644 --- a/docs/formatting.md +++ b/docs/formatting.md @@ -19,13 +19,22 @@ multi_formatter_binary( ) ``` +Finally, we recommend an alias in the root BUILD file: + +```starlark +alias( + name = "format", + actual = "//tools:format", +) +``` + ## Usage ### One-time re-format all files Assuming you installed with the typical layout: -`bazel run tools:format` +`bazel run //:format` > Note that mass-reformatting can be disruptive in an active repo. > You may want to instruct developers with in-flight changes to reformat their branches as well, to avoid merge conflicts. @@ -35,7 +44,7 @@ Assuming you installed with the typical layout: ### Re-format specific file(s) -`bazel run tools:format some/file.md other/file.json` +`bazel run //:format some/file.md other/file.json` ### Install as a pre-commit hook @@ -44,10 +53,10 @@ If you use [pre-commit.com](https://pre-commit.com/), add this in your `.pre-com ```yaml - repo: local hooks: - - id: bazel-super-formatter + - id: aspect_rules_lint name: Format language: system - entry: bazel run //tools:format + entry: bazel run //:format files: .* ``` @@ -58,7 +67,7 @@ If you don't use pre-commit, you can just wire directly into the git hook, howev this option will always run the formatter over all files, not just changed files. ```bash -$ echo "bazel run //tools:format -- --mode check" >> .git/hooks/pre-commit +$ echo "bazel run //:format -- --mode check" >> .git/hooks/pre-commit $ chmod u+x .git/hooks/pre-commit ``` @@ -66,4 +75,4 @@ $ chmod u+x .git/hooks/pre-commit This will exit non-zero if formatting is needed. You would typically run the check mode on CI. -`bazel run //tools:format -- --mode check` +`bazel run //:format -- --mode check` diff --git a/example/BUILD.bazel b/example/BUILD.bazel index adc0bf1b..4747f824 100644 --- a/example/BUILD.bazel +++ b/example/BUILD.bazel @@ -1,10 +1,6 @@ load("@aspect_rules_js//js:defs.bzl", "js_library") -load("@aspect_rules_lint//lint:shellcheck.bzl", "shellcheck_binary") load("@npm//:defs.bzl", "npm_link_all_packages") -load("@npm//:eslint/package_json.bzl", eslint_bin = "bin") -load("@rules_java//java:defs.bzl", "java_binary") load("@rules_python//python:pip.bzl", "compile_pip_requirements") -load("@rules_python//python/entry_points:py_console_script_binary.bzl", "py_console_script_binary") package(default_visibility = ["//visibility:public"]) @@ -26,15 +22,6 @@ exports_files( visibility = ["//visibility:public"], ) -# We can test that it works with: -# bazel run :flake8 -- --help -py_console_script_binary( - name = "flake8", - pkg = "@pip//flake8:pkg", -) - -eslint_bin.eslint_binary(name = "eslint") - js_library( name = "eslintrc", srcs = [".eslintrc.cjs"], @@ -44,11 +31,18 @@ js_library( ], ) -java_binary( - name = "pmd", - main_class = "net.sourceforge.pmd.PMD", - runtime_deps = ["@net_sourceforge_pmd"], +# NB: this alias does NOT cause Bazel's Loading phase to load the tools/BUILD file. +# That's important as we don't want users to wait for "Eager fetching" for ~EVERY language which +# that build file loads from. +# Demonstration: we'll build the js_library above, then build this format alias, and see that many +# more repositories were fetched for the latter: +# % export T=$(mktemp -d) +# % bazel --output_base=$T build :eslintrc; ls $T/external > one +# % bazel --output_base=$T build :format; ls $T/external > two +# % wc -l one two +# 738 one +# 936 two +alias( + name = "format", + actual = "//tools:format", ) - -# bazel run :shellcheck -- --help -shellcheck_binary(name = "shellcheck") diff --git a/example/tools/BUILD b/example/tools/BUILD index 4ffb2987..38dbadfe 100644 --- a/example/tools/BUILD +++ b/example/tools/BUILD @@ -5,7 +5,11 @@ we don't want to trigger eager fetches of these for builds that don't want to ru """ load("@aspect_rules_lint//format:defs.bzl", "multi_formatter_binary") +load("@aspect_rules_lint//lint:shellcheck.bzl", "shellcheck_binary") +load("@npm//:eslint/package_json.bzl", eslint_bin = "bin") load("@npm//:prettier/package_json.bzl", prettier = "bin") +load("@rules_java//java:defs.bzl", "java_binary") +load("@rules_python//python/entry_points:py_console_script_binary.bzl", "py_console_script_binary") package(default_visibility = ["//:__subpackages__"]) @@ -90,6 +94,24 @@ java_binary( runtime_deps = ["@maven//:org_scalameta_scalafmt_cli_2_13"], ) +# We can test that it works with: +# bazel run :flake8 -- --help +py_console_script_binary( + name = "flake8", + pkg = "@pip//flake8:pkg", +) + +eslint_bin.eslint_binary(name = "eslint") + +java_binary( + name = "pmd", + main_class = "net.sourceforge.pmd.PMD", + runtime_deps = ["@net_sourceforge_pmd"], +) + +# bazel run :shellcheck -- --help +shellcheck_binary(name = "shellcheck") + multi_formatter_binary( name = "format", go = "@go_sdk//:bin/gofmt", diff --git a/example/tools/lint.bzl b/example/tools/lint.bzl index 876b9f6d..6a52dfba 100644 --- a/example/tools/lint.bzl +++ b/example/tools/lint.bzl @@ -13,21 +13,21 @@ buf = buf_lint_aspect( ) eslint = eslint_aspect( - binary = "@@//:eslint", + binary = "@@//tools:eslint", config = "@@//:eslintrc", ) eslint_test = make_lint_test(aspect = eslint) flake8 = flake8_aspect( - binary = "@@//:flake8", + binary = "@@//tools:flake8", config = "@@//:.flake8", ) flake8_test = make_lint_test(aspect = flake8) pmd = pmd_aspect( - binary = "@@//:pmd", + binary = "@@//tools:pmd", rulesets = ["@@//:pmd.xml"], ) @@ -39,7 +39,7 @@ ruff = ruff_aspect( ) shellcheck = shellcheck_aspect( - binary = "@@//:shellcheck", + binary = "@@//tools:shellcheck", config = "@@//:.shellcheckrc", )