rules_pkg/developers.md

4.8 KiB

rules_pkg - Information for Developers

NOTE: This is perpetually a work in progress. We will revise it as we need.

PR guidelines

Please discuss proposed major features and structural changes before sending a PR. The best way to do that is to do one or more of the following.

  1. Create an issue
  2. Discuss it in the issue or in rules-pkg-discuss@googlegroups.com
  3. Bring it up at the monthly engineering meeting. (See #325 for details)[https://github.com/bazelbuild/rules_pkg/issues/325].

A few small PRs are better than one big one.

If you need to refactor a lot of code before you can add new behavior, please send a refactoring PR first (which should not add or change tests), then send a smaller change that implements the new behavior.

Functionality

General solutions are better than package specific ones.

  • Favor solutions that can be reused for all package formats. We will not accept contributions that add specific features to one package format when they could be incorporated into existing pkg_files and related rules such that they could be used by all formats.
  • We are moving towards aligning attribute names across rules. If we want backwards compatibility with older versions, do that in the macro wrapper.

Tests

  • All behavioral changes require tests. Even if you are fixing what is most likely a bug, please add a test that would have failed before the fix and passes after.
  • Aim for minimal tests. Favor unit tests over integration tests. It is better to have many small tests rather than one that checks many conditions.
  • Continuous testing is defined under .bazel_ci.

Portability

  • All of the code must work on Linux, macOS, and Windows. Other OSes are optional, but we do not have CI to ensure it. If you are making changes that require something specifically for portability, your change should include inline comments about why, so a future maintainer does not accidentally revert it.
  • Some features can not work on all platforms. Favor solutions that allow auto-skipping tests that are platform specific rather than requiring exclusion lists in CI.

Major features require commitment

We would love to have features like an MSI builder or macOS application support. Before accepting a PR for something like that, we want to know that your organization will commit to maintaining that feature and responding to issues.

About the code

Repository structure: run time vs build time

  • pkg/... contains what is needed at run time. Everything else is not part of the packaged distribution releases.
  • docs/... is served as github pages. We mostly generate that rather than edit by hand.
  • distro/... contains the rules to create the distribution package.
  • examples/... contains runnable examples which serve as additional documentation. These are tested in CI.
  • tests/... contains unit and integration tests. It is a distinct folder so that it is not needed in the distribution runtime.
  • contains shims for the .bzl files in pkg/*.bzl. They add backwards compatibility to to older releases.

Starlark style

  • We are moving towards fully generated docs. If you touch an attribute, you must update the docstring to explain it.
  • Add docstrings args defined in macros. We are targeting the future time when stardoc can build unified documentation for the rule and the wrapper.
  • Actions should not write quoted strings to command lines. If your rule must pass file paths to another program, write the paths to an intermediate file and pass that as an arg to the other program. See #214.
  • Run buildifier on you .bzl files before sending a PR.

Python style

  • We use Python 3. We are not trying to support Python 2 at all.
  • Always import with a full path from the root of the source tree.
  • Compatibility back to python 3.6 (for CentOS 7 and Ubuntu 18.04 LTS) is required.

Dependencies

  • No new dependencies on language toolchains. We take a dependency on Python because it is generally available on all OSes.
  • No new python package dependencies. Use only what is part of the core distribution.
  • Other new dependencies are strongly discouraged. The exception is that we may take dependencies on other modules maintained by the Bazel team.

Write for vendoring the source tree.

We presume that some users will vendor this entire rule set into their source tree and want to test it from their WORKSPACE. Towards that end we try to minimize the places where we assume the path to any package is absolute from WORKSPACE. See tests/package_naming_aggregate_test.sh for an example of how we can write a sh_test that works after re-rooting the sources.