-
Notifications
You must be signed in to change notification settings - Fork 207
Expand file tree
/
Copy pathCONTRIBUTING
More file actions
142 lines (97 loc) · 8.93 KB
/
CONTRIBUTING
File metadata and controls
142 lines (97 loc) · 8.93 KB
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
# Contributing to OpenInference
## Read This First
We are not actively accepting feature contributions right now.
You can still open an issue or PR, but please do so knowing there is a good chance we close it or never get to it.
We appreciate the impulse to contribute. This project is used by many people and we are trying to keep scope, quality, and direction tightly controlled. Limiting what we accept is how we do that.
## What We Are Most Likely To Accept
- Small, focused bug fixes.
- Small reliability fixes.
- Small performance improvements.
- Tightly scoped maintenance work that clearly improves the project without changing its direction.
## What We Are Least Likely To Accept
- Large PRs.
- Drive-by feature work.
- Opinionated rewrites.
- Anything that expands product scope without us asking for it first.
A 1,000+ line PR full of new features is very likely to be closed without review. Please don't put yourself through that.
## Opening A PR
- Keep it small.
- Explain exactly what changed.
- Explain exactly why the change should exist.
- Do not mix unrelated fixes together.
- If the change affects emitted spans (e.g. new attributes, renamed fields, or a different span hierarchy), include a screenshot showing the updated trace output.
- If we have to guess what changed, we are much less likely to review it.
## Issues First
If you are thinking about a non-trivial change, open an issue first.
That does not guarantee we will want the PR, but it gives us a chance to align before you invest the effort.
An open issue is not an invitation to submit a PR. Even if an issue is labeled or acknowledged, please wait for explicit confirmation from a maintainer before starting work on it. We track many issues we are not yet ready to address, and unsolicited PRs for them will likely be closed.
## Expectations
Opening a PR does not create an obligation on our side. We may close it, ask you to shrink it, or reimplement the idea ourselves later.
We know that can be frustrating. We would rather be upfront about it than waste your time with false encouragement.
## Code of Conduct
This project has adopted the [Contributor Covenant Code of Conduct](./CODE_OF_CONDUCT.md). By participating, you are expected to uphold this code and actions that violate it will not be tolerated.
## Branch Organization
Submit all changes to `main`.
Code that lands in `main` must be compatible with the latest stable release. It may contain additional features but no breaking changes. We should be able to release a new minor version from the tip of `main` at any time.
## Bugs
We use GitHub issues to track bugs. We keep a close eye on this and try to make it clear when we have an internal fix in progress. Before filing a new task, try to make sure your problem doesn't already exist.
## Pull Requests
Before submitting a pull request, please make sure the following is done:
- Fork the repository and create your branch from `main`.
- Follow the development guide in [python](./python/DEVELOPMENT.md) or [javascript](./js/DEVELOPMENT.md) to setup your local environment.
- **Install pre-commit hooks** (one-time setup):
```bash
pip install pre-commit
pre-commit install
```
Hooks run automatically on `git commit` and enforce `ruff` formatting/linting (Python) and `oxfmt`/`oxlint` (JS/TS). You can also run them manually at any time:
```bash
pre-commit run --all-files
```
- **Run formatters and linters** manually with `make`:
```bash
make format # format all Python + JS/TS files
make lint # lint all Python + JS/TS files
```
- If you've fixed a bug or added code that should be tested, add tests!
- Ensure test suite passes.
- Make sure your code is formatted by a formatter.
- Make sure your code lints.
- Run type checking.
### Pull Request (PR) Descriptions
A PR description is a public record of what change is being made and why it was made. It will become a permanent part of our version control history, and will possibly be read by many people other than your reviewers over the years. Follow the following guidelines when writing a PR description:
- Title: The title must conform to [conventional commit](https://www.conventionalcommits.org/en/v1.0.0/#summary) format and must sufficiently describe the change. Since PR titles are used to form release notes, titles with generic or non-descriptive content ("Fix build.", "Add patch.") are not allowed.
- Issue: The first line of the description should contain a reference to the issue that the PR is solving. For example, `fixes #1234` or `resolves #1234`. While this is not required for urgent fixes, it is required for all other PRs so that the issue is clearly tracked and triaged by a core team member.
- Description: The first line should be a short, focused summary, while the rest of the description should fill in the details and include any supplemental information a reader needs to understand the change holistically. It might include a brief description of the problem that's being solved, and why this is the best approach. If there are any shortcomings to the approach, they should be mentioned.
- Videos and screenshots: Highlight the changes in the UI. These should be supplemental to the text description, not a replacement for it.
- Code Snippets: If the PR is changing an API, include code snippets to highlight the changes. This will expedite a reader's ability to construct the right API calls if they are interested in doing so. These should be supplemental to the text description, not a replacement for it.
### Small Simple Pull Requests
Small, simple pull requests are important because they are:
- Reviewed more quickly
- Reviewed more thoroughly
- Less likely to introduce bugs
- Easier to merge
- Easier to design well
- Less blocking on reviews
- Simpler to roll back
Note that reviewers have discretion to reject your change outright for the sole reason of it being too large. Usually they will thank you for your contribution but request that you somehow make it into a series of smaller changes. It can be a lot of work to split up a change after you've already written it, or require lots of time arguing about why the reviewer should accept your large change. It's easier to just write small PRs in the first place.
For a comprehensive guide on how to write small PRs, see this [guide](https://github.com/google/eng-practices/blob/master/review/developer/small-cls.md)
## Code Reviews
A code review is a process where someone other than the author(s) of a piece of code examines that code. Code committed to the codebase is both the responsibility of the author and the reviewer. Code reviews should look at:
- Design: Is the code well-designed and appropriate for your system?
- Functionality: Does the code behave as the author likely intended? Is the way the code behaves good for its users?
- Complexity: Could the code be made simpler? Would another developer be able to easily understand and use this code when they come across it in the future? Beware of over-engineering. The code should solve problems that need to be solved _now_, and not problems that the code author speculates _might_ need to be solved in the future.
- Tests: Does the code have correct and well-designed automated tests?
- Naming: Did the developer choose clear names for variables, classes, methods, etc.?
- Comments: Are the comments clear and useful? Note that comments are most useful when they explain _why_ the code exists.
- Style: Does the code follow our style guides? Note, in most cases, style nits should be avoided and be enforced entirely by automated tooling. However some stylistic decisions can be discussed if it impacts readability and complexity.
- Documentation: Did the developer also update relevant documentation?
All of the above are grounds for a reviewer to request changes in a PR. Consensus should be reached to the best of the abilities of the author(s) and reviewer. However, if consensus cannot be reached between the two parties, the review should be escalated to the technical lead.
### Code Review Conduct
- Be kind.
- Explain your reasoning.
- Balance giving explicit directions with just pointing out problems and letting the developer decide.
- Encourage developers to simplify code or add code comments instead of just explaining the complexity to you.
Remember that you are both on the same team and are working towards the same goal. The goal is not to make the code perfect, but to make it better. Make sure your review comments are constructive and actionable. Treat the code contribution as a communal artifact under construction rather than a personal creation under evaluation. For a comprehensive guide on how to provide effective PR feedback, see this [guide](https://google.github.io/eng-practices/review/reviewer/comments.html).
## Contributor License Agreement (CLA)
In order to accept your pull request, we need you to submit a CLA. You only need to do this once. Simply reply to your first pull request with `I have read the CLA Document and I hereby sign the CLA`. Your signature will be recorded automatically in the `cla` branch.