Skip to content

Commit 437b4e4

Browse files
authored
chore(docs): update contribution guideline (#2924)
1 parent ca51aaa commit 437b4e4

File tree

1 file changed

+100
-57
lines changed

1 file changed

+100
-57
lines changed

CONTRIBUTING

Lines changed: 100 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,99 +1,142 @@
1-
# Contributing to Arize-Phoenix
1+
# Contributing to OpenInference
22

3-
`openinference` is an open source project that is under active development that is being used by Arize AI customers as well as the larger AI community. This document aims to make it easy for anyone to contribute to the project.
3+
## Read This First
4+
5+
We are not actively accepting feature contributions right now.
6+
7+
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.
8+
9+
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.
10+
11+
## What We Are Most Likely To Accept
12+
13+
- Small, focused bug fixes.
14+
- Small reliability fixes.
15+
- Small performance improvements.
16+
- Tightly scoped maintenance work that clearly improves the project without changing its direction.
17+
18+
## What We Are Least Likely To Accept
19+
20+
- Large PRs.
21+
- Drive-by feature work.
22+
- Opinionated rewrites.
23+
- Anything that expands product scope without us asking for it first.
24+
25+
A 1,000+ line PR full of new features is very likely to be closed without review. Please don't put yourself through that.
26+
27+
## Opening A PR
28+
29+
- Keep it small.
30+
- Explain exactly what changed.
31+
- Explain exactly why the change should exist.
32+
- Do not mix unrelated fixes together.
33+
- 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.
34+
- If we have to guess what changed, we are much less likely to review it.
35+
36+
## Issues First
37+
38+
If you are thinking about a non-trivial change, open an issue first.
39+
40+
That does not guarantee we will want the PR, but it gives us a chance to align before you invest the effort.
41+
42+
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.
43+
44+
## Expectations
45+
46+
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.
47+
48+
We know that can be frustrating. We would rather be upfront about it than waste your time with false encouragement.
449

550
## Code of Conduct
651

752
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.
853

954
## Branch Organization
1055

11-
Submit all code changes to `main` and documentation changes to the `docs` branch.
56+
Submit all changes to `main`.
1257

1358
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.
1459

1560
## Bugs
1661

17-
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 doesnt already exist.
62+
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.
1863

1964
## Pull Requests
2065

21-
The core team is monitoring for pull requests. We will review your pull request and either merge it, request changes to it, or close it with an explanation.
22-
23-
Before submitting a pull request, please make sure the following is done. We expect code authors to test their pull requests well-enough that they work correctly by the time they get to code review. (Note that `npm` commands below are run under the `./app` directory.)
24-
25-
- Fork the repository and create your branch from `main`.
26-
- Follow the development guide in [python](./python/DEVELOPMENT.md) or [javascript](./js/DEVELOPMENT.md) to setup your local environment.
27-
- **Install pre-commit hooks** (one-time setup):
28-
```bash
29-
pip install pre-commit
30-
pre-commit install
31-
```
32-
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:
33-
```bash
34-
pre-commit run --all-files
35-
```
36-
- **Run formatters and linters** manually with `make`:
37-
```bash
38-
make format # format all Python + JS/TS files
39-
make lint # lint all Python + JS/TS files
40-
```
41-
- If you've fixed a bug or added code that should be tested, add tests!
42-
- Ensure test suite pass.
43-
- Make sure your code is formatted by a formatter
44-
- Make sure to your code lints
45-
- Run type checking
66+
Before submitting a pull request, please make sure the following is done:
67+
68+
- Fork the repository and create your branch from `main`.
69+
- Follow the development guide in [python](./python/DEVELOPMENT.md) or [javascript](./js/DEVELOPMENT.md) to setup your local environment.
70+
- **Install pre-commit hooks** (one-time setup):
71+
```bash
72+
pip install pre-commit
73+
pre-commit install
74+
```
75+
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:
76+
```bash
77+
pre-commit run --all-files
78+
```
79+
- **Run formatters and linters** manually with `make`:
80+
```bash
81+
make format # format all Python + JS/TS files
82+
make lint # lint all Python + JS/TS files
83+
```
84+
- If you've fixed a bug or added code that should be tested, add tests!
85+
- Ensure test suite passes.
86+
- Make sure your code is formatted by a formatter.
87+
- Make sure your code lints.
88+
- Run type checking.
4689

4790
### Pull Request (PR) Descriptions
4891

4992
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:
5093

51-
- Title: The tile 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.
52-
- 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.
53-
- 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 thats being solved, and why this is the best approach. If there are any shortcomings to the approach, they should be mentioned.
54-
- Videos and screenshots: Highlight the changes in the UI. These should be supplemental to the text description, not a replacement for it.
55-
- 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.
94+
- 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.
95+
- 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.
96+
- 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.
97+
- Videos and screenshots: Highlight the changes in the UI. These should be supplemental to the text description, not a replacement for it.
98+
- 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.
5699

57100
### Small Simple Pull Requests
58101

59102
Small, simple pull requests are important because they are:
60103

61-
- Reviewed more quickly
62-
- Reviewed more thoroughly
63-
- Less likely to introduce bugs
64-
- Easier to merge.
65-
- Easier to design well
66-
- Less blocking on reviews
67-
- Simpler to roll back
104+
- Reviewed more quickly
105+
- Reviewed more thoroughly
106+
- Less likely to introduce bugs
107+
- Easier to merge
108+
- Easier to design well
109+
- Less blocking on reviews
110+
- Simpler to roll back
68111

69112
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.
70113

71114
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)
72115

73-
## Contributor License Agreement (CLA)
74-
75-
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.
76-
77116
## Code Reviews
78117

79118
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:
80119

81-
- Design: Is the code well-designed and appropriate for your system?
82-
- Functionality: Does the code behave as the author likely intended? Is the way the code behaves good for its users?
83-
- 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.
84-
- Tests: Does the code have correct and well-designed automated tests?
85-
- Naming: Did the developer choose clear names for variables, classes, methods, etc.?
86-
- Comments: Are the comments clear and useful? Note that comments are most useful when they explain _why_ the code exists.
87-
- 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.
88-
- Documentation: Did the developer also update relevant documentation?
120+
- Design: Is the code well-designed and appropriate for your system?
121+
- Functionality: Does the code behave as the author likely intended? Is the way the code behaves good for its users?
122+
- 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.
123+
- Tests: Does the code have correct and well-designed automated tests?
124+
- Naming: Did the developer choose clear names for variables, classes, methods, etc.?
125+
- Comments: Are the comments clear and useful? Note that comments are most useful when they explain _why_ the code exists.
126+
- 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.
127+
- Documentation: Did the developer also update relevant documentation?
89128

90129
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.
91130

92131
### Code Review Conduct
93132

94-
- Be kind.
95-
- Explain your reasoning.
96-
- Balance giving explicit directions with just pointing out problems and letting the developer decide.
97-
- Encourage developers to simplify code or add code comments instead of just explaining the complexity to you.
133+
- Be kind.
134+
- Explain your reasoning.
135+
- Balance giving explicit directions with just pointing out problems and letting the developer decide.
136+
- Encourage developers to simplify code or add code comments instead of just explaining the complexity to you.
98137

99138
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).
139+
140+
## Contributor License Agreement (CLA)
141+
142+
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.

0 commit comments

Comments
 (0)