From 987b8311f57fb8fd68b35852b1a83eab3a5b87a4 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Wed, 23 Apr 2025 15:10:39 -0700 Subject: [PATCH 1/6] docs: document some of our project styles/conventions --- CONTRIBUTING.md | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 17558e1b23..134e35b484 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -173,6 +173,47 @@ The `legacy_foo` arg was removed ::: ``` +## Style and idioms + +For the most part, we just accept whatever the code formatters do, so there +isn't much style to enforce. + +Some miscellanous style, idioms, and conventions we have are: + +### Markdown/Sphinx Style + +* Use colons (e.g. `:::`) for prose sections of text. +* Use backticks for code blocks. +* Max line length: 100 + +### BUILD/bzl Style + +* When a macro generates public targets, use a dot (`.`) to separate the + user-provided name from the generted name. e.g. `foo(name="x")` generates + `x.test`. The `.` is our convention to communicate that it's a generated + target, and thus one should look for `name="x"` when searching for the + definition. +* The different build phases and interface/implementation pieces of rules should + be in different files. Stated another way: loading code for thing shouldn't + incur loading code that isn't relevant. + * Providers should be in their own files. This allows implementing a custom + rule that implements the provider without loading an existing + implementation. + * The repository phase shouldn't `load()` code that is only relevant to the + loading-phase or subsequent phases. Vice-versa, loading-phase code should + not load any repository-phase code. + * Generally, one rule per file. The goal is that defining an e.g. library + shouldn't incur loading all the code for binaries, tests, packaging, etc. +* The public access point for anything should be a separate file that re-exports + the specific symbols that are public. This ensures our public API is well + defined and prevents accidentally exposing a package-private symbol as a + public symbol. +* Repository rules should have name ending in `_repo`. This helps distinguish + them from regular rules. +* Each bzlmod extension, the "X" of `use_repo("//foo:foo.bzl", "X")` should be + in its own file. The path given in the `use_repo()` expression is the identity + Bazel uses and cannot be changed. + ## Generated files Some checked-in files are generated and need to be updated when a new PR is From 852b95e3195f405180f9c88e3dd0dcc634193af6 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Wed, 23 Apr 2025 16:27:20 -0700 Subject: [PATCH 2/6] clarify prose example --- CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 134e35b484..1fdf8a1982 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -182,9 +182,9 @@ Some miscellanous style, idioms, and conventions we have are: ### Markdown/Sphinx Style -* Use colons (e.g. `:::`) for prose sections of text. +* Use colons for prose sections of text, e.g. `:::{note}`, not backticks. * Use backticks for code blocks. -* Max line length: 100 +* Max line length: 100. ### BUILD/bzl Style From 572a35221ca0d2f8edd4e1c8144060b21eb76c02 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Thu, 24 Apr 2025 12:30:24 -0700 Subject: [PATCH 3/6] rephrase public access doc --- .editorconfig | 13 +++++++++++++ CONTRIBUTING.md | 12 ++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) create mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000000..3eaf31f3dc --- /dev/null +++ b/.editorconfig @@ -0,0 +1,13 @@ +# Unix-style newlines with a newline ending every file +[*] +end_of_line = lf +insert_final_newline = true + +# Set default charset +[*] +charset = utf-8 + +# 4 space indentation +[*.{py,bzl}] +indent_style = space +indent_size = 4 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1fdf8a1982..2e9925020b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -204,10 +204,14 @@ Some miscellanous style, idioms, and conventions we have are: not load any repository-phase code. * Generally, one rule per file. The goal is that defining an e.g. library shouldn't incur loading all the code for binaries, tests, packaging, etc. -* The public access point for anything should be a separate file that re-exports - the specific symbols that are public. This ensures our public API is well - defined and prevents accidentally exposing a package-private symbol as a - public symbol. +* Separate files should be used to expose public APIs. This ensures our public + API is well defined and prevents accidentally exposing a package-private + symbol as a public symbol. + + :::{note} + The public API file's docstring becomes part of the user-facing docs. That + file's docstring must be used for module-level API documentation. + ::: * Repository rules should have name ending in `_repo`. This helps distinguish them from regular rules. * Each bzlmod extension, the "X" of `use_repo("//foo:foo.bzl", "X")` should be From 9590d56ebb1a9480b5506946cf7ba424ef83ac64 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Thu, 24 Apr 2025 12:39:17 -0700 Subject: [PATCH 4/6] rephase build phase doc, clarify provider/rule definition --- CONTRIBUTING.md | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2e9925020b..b087119dc6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -193,17 +193,21 @@ Some miscellanous style, idioms, and conventions we have are: `x.test`. The `.` is our convention to communicate that it's a generated target, and thus one should look for `name="x"` when searching for the definition. -* The different build phases and interface/implementation pieces of rules should - be in different files. Stated another way: loading code for thing shouldn't - incur loading code that isn't relevant. - * Providers should be in their own files. This allows implementing a custom - rule that implements the provider without loading an existing - implementation. - * The repository phase shouldn't `load()` code that is only relevant to the - loading-phase or subsequent phases. Vice-versa, loading-phase code should - not load any repository-phase code. - * Generally, one rule per file. The goal is that defining an e.g. library - shouldn't incur loading all the code for binaries, tests, packaging, etc. +* The different build phases shouldn't load code that defines objects that + aren't valid for their phase. e.g. + * The bzlmod phase shouldn't load code defining regular rules or providers. + * The repository phase shouldn't load code defining module extensions, regular + rules, or providers. + * The loading phase shouldn't load code defining module extensions or + repository rules. + * Loading utility libraries or generic code is OK, but should strive to load + code that is usable for its phase. e.g. loading-phase code shouldn't + load utility code that is predominately only usable to the bzlmod phase. +* Providers should be in their own files. This allows implementing a custom rule + that implements the provider without loading a specific implementation. +* One rule per file is preferred, but not required. The goal is that defining an + e.g. library shouldn't incur loading all the code for binaries, tests, + packaging, etc; things that may be niche or uncommonly used. * Separate files should be used to expose public APIs. This ensures our public API is well defined and prevents accidentally exposing a package-private symbol as a public symbol. From 663cb821a689cc6505b0ca3e0c5ce54a60bb4765 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 29 Apr 2025 22:52:47 +0900 Subject: [PATCH 5/6] add max_line_length --- .editorconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/.editorconfig b/.editorconfig index 3eaf31f3dc..7c7d6f5af5 100644 --- a/.editorconfig +++ b/.editorconfig @@ -2,6 +2,7 @@ [*] end_of_line = lf insert_final_newline = true +max_line_length = 100 # Set default charset [*] From f5a83d1f38cd0eae807317101af2b12be27bf462 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 29 Apr 2025 22:54:26 +0900 Subject: [PATCH 6/6] update .editorconfig --- .editorconfig | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.editorconfig b/.editorconfig index 7c7d6f5af5..26bb52ffac 100644 --- a/.editorconfig +++ b/.editorconfig @@ -2,12 +2,15 @@ [*] end_of_line = lf insert_final_newline = true -max_line_length = 100 # Set default charset [*] charset = utf-8 +# Line width +[*] +max_line_length = 100 + # 4 space indentation [*.{py,bzl}] indent_style = space