forked from bazelbuild/rules_pkg
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fixes bazelbuild#214. More accurately, it moves the intent of bazelbuild#214 into this guide. This feels appropriate because it is not a single problem, but rather a general principal.
- Loading branch information
Showing
1 changed file
with
71 additions
and
17 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,28 +1,82 @@ | ||
# rules_pkg - Information for Developers | ||
|
||
NOTE: This is a work in progress. It is only an outline at this point. | ||
NOTE: This is perpetually a work in progress. We will revise it as we need. | ||
|
||
TBD: Introduction | ||
## PR guidelines | ||
|
||
## General | ||
Please discuss proposed major features and structural changes before sending a | ||
PR. The best way to do that is to | ||
|
||
- tests | ||
- OS portability | ||
- minimize tests | ||
- repo structure | ||
- more at build time than in the distribution | ||
- some features do not work on all platforms | ||
1. Create an an issue | ||
1. Discuss it in the issue or in [email protected] | ||
1. Bring it up at the monthly engineering meeting. (See #325 for | ||
details)[https://github.com/bazelbuild/rules_pkg/issues/325]. | ||
|
||
## Starlark style | ||
### A few small PRs are better than one big one. | ||
|
||
- using analysis tests | ||
- docstrings in attributes | ||
If you need to refactor a lot of code before you can add new behavior, please | ||
send a refactoring CL first (which should not add or change tests), then send a | ||
smaller change that implements the new behavior. | ||
|
||
## Python style | ||
## Functionality | ||
|
||
- import style as per #399 | ||
- ... | ||
### General solutions are better than package specific ones. | ||
|
||
## Continuous testing | ||
- 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. | ||
|
||
- under .bazel_ci | ||
### 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. | ||
|
||
### 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 lings. 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)[https://github.com/bazelbuild/rules_pkg/issues/214]. | ||
|
||
### 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. |