General Rules

• Google Python style, 80 column limit
• Commit messages should have the module name or recipe prefix instead of something like “fix:” or “[fix]”
  • Yes: “checkout: Fix spelling error”
  • No: “fix: Fix spelling error”
  • No: “[checkout] Fix spelling error”
• Use Python type annotations where possible
  • Use ‘list’ and ‘dict’ instead of ‘typing.List’ and ‘typing.Dict’
  • Use ‘from typing import ...’ and ‘from collections.abc import ...’ instead of ‘import typing’ and ‘import collections.abc’
  • Use ‘from future import annotations’
  • Function and method arguments should use generic types if possible, like ‘Sequence’
  • Return values should use specific types, like ‘list’
  • In most cases, don't hide type annotation imports under ‘if TYPE_CHECKING:’
    • Yes: from typing import Callable
    • No: from typing import TYPE_CHECKING ... if TYPE_CHECKING: from typing import Callable
    • Exception: imports used only for annotations that look like “from RECIPE_MODULES.repo.module import ...” where “repo/module” is not in ‘DEPS’ should be hidden behind ‘if TYPE_CHECKING:’—this will allow a reference to a type defined in another module without taking a full dependency on that module
      • If “repo/module” is in ‘DEPS’, then don't hide with ‘if TYPE_CHECKING:’
  • Make sure any “if TYPE_CHECKING:” lines that do exist have a “# pragma: no cover” comment
• Make sure things pass pylint
• Recipe testing often requires ‘test_data’ or ‘step_test_data’ arguments in what otherwise looks like production code—leave these arguments alone
• Setting properties on a StepPresentation modifies global data, so don't remove apparent no-ops that just modify a ‘properties’ member of an object
• Don't include comments on import lines
• Don't have any trailing whitespace at the end of lines
• The ‘api’ object passed into a recipe's ‘RunSteps’ function is of type ‘recipe_api.RecipeScriptApi’ and it returns ‘RawResult | None’
• The ‘api’ object passed into a recipe's ‘GenTests’ function is of type ‘recipe_test_api.RecipeTestApi’ and it returns ‘Iterator[recipe_test_api.TestData]’
• All recipes must define both ‘RunSteps’ and ‘GenTests’
• Indentation in docstrings should be independent of the length of the variable name
  • Yes: Args: very_long_variable_name: Description takes multiple lines.
  • No: Args: very_long_variable_name: Description takes multiple lines.
• Use dataclasses instead of attrs
  • Exceptions ok for when using attrs features not in dataclasses
• Don't import individual classes or variables—always import modules, and never use ‘from import *’
  • Exceptions:
    • from pathlib import Path
    • from collections.abc import
    • from typing import
    • from PB. import InputProperties
• All imports should be at top-level, even if they're only used in GenTests()
• When importing a module under “PB”, import “as _pb”
  • Yes: from PB.recipe_engine import result as result_pb
  • No: from PB.recipe_engine import result

Module Dependencies

• Recipe modules contain a single class that is a subclass of recipe_api.RecipeApi
• Recipes can list their dependencies in a module-level ‘DEPS’ variable
  • Dependencies without a “/” refer to the current repo, and if they do have a “/” the part in front is a repository name
  • The remote for that repository can be found in “infra/config/recipes.cfg”
• Recipe modules list their dependencies in a ‘DEPS’ variable in ‘init.py’
• Recipes can access an object that is an instance of the subclass of recipe_api.RecipeApi using ‘api.’
• Recipe modules can access that same object using ‘self.m.’
• If necessary, modules themselves can be imported with lines like this: ‘from RECIPE_MODULES.. import api as _api’
  • Prefer this structure over other ways to import modules directly
• Prefer type annotations based on the api object over imports
  • Yes: def RunTests(api: recipe_api.RecipeScriptApi): checkout: api.checkout.CheckoutContext = ...
  • No: from RECIPE_MODULES.pigweed.checkout import api as checkout_api def RunTests(api: recipe_api.RecipeScriptApi): checkout: checkout_api.CheckoutContext = ...

Testing

• After making a change, run ‘./presubmit.py’ to check that everything still works
• Also run ‘shac fmt’ to adjust any formatting
• Run ‘shac check’ to check for lint issues