Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Contributing guide #1501

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 167 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,170 @@
# Contributing to OpenTelemetry PHP

## Introduction

Welcome to the OpenTelemetry PHP repository! 🎉

Thank you for considering contributing to this project. Whether you're fixing a bug, adding new features, improving documentation, or reporting an issue, we appreciate your help in making OpenTelemetry better.

This repository is part of the OpenTelemetry ecosystem, which provides observability tooling for distributed systems. Your contributions help enhance the PHP ecosystem for OpenTelemetry users worldwide.

If you have any questions, feel free to ask in the community channels. We’re happy to help! 😊

## Pre-requisites

Before getting started, ensure you have the following installed:

- **PHP** (8.1 or higher) – [Install PHP](https://www.php.net/downloads)
- **Composer** – [Install Composer](https://getcomposer.org/)
- **Docker & Docker Compose** – [Install Docker](https://docs.docker.com/engine/install/)
- **Make** (for running development tasks)

Additional Notes:
- Windows users may need [Git Bash](https://gitforwindows.org/) for better compatibility.
- Consider using [phpenv](https://github.com/phpenv/phpenv) for managing multiple PHP versions.

## Workflow

We follow a structured workflow to ensure smooth collaboration:

### Branch Naming Convention
- **Feature branches**: `feature/<short-description>`
- **Bugfix branches**: `fix/<short-description>`
- **Documentation updates**: `docs/<short-description>`

### Commit Message Format
- Use descriptive commit messages (e.g., `fix(tracing): resolve issue with span context`)
- Follow [Conventional Commits](https://www.conventionalcommits.org/) where possible.
Comment on lines +30 to +37
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't currently have a standard or practise for branch naming or commit messages. I assume this has come from another repository? Is it considered best-practise for all OpenTelemetry projects?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if there any general format though, it can be removed if there is no standard yet for this repo though, or can be adopted this depending on what is tenable


### Pull Request Guidelines
- Fork the repository and create a new branch.
- Follow the coding guidelines before submitting your PR.
- Ensure tests pass locally before pushing.
- Link relevant issues in the PR description.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does each PR require an issue? I have a trivial PR and will make an issue for it, but it'd be good if there were guidance in case there are some circumstances one isn't required

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a contributor myself, so I guess the maintainers would have better knowledge to give a response to this

cc @brettmc @ChrisLightfootWild

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to receive PRs without a corresponding issue, provided the PR provides some detail.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm the same as @brettmc. Although there's a risk that contributors spend more time on a PR that would not have been necessary with some up-front discussion in an issue/other channel.



## Local Run/Build
Copy link
Contributor

@spadger spadger Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi

I did my first work on this repo a few days ago, and the makefile doesn't work straight after a clone because of this line

include .env

I see what it does now, but some guidance on how to provision one would have been helpful

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How to set up .env is at the very top of DEVELOPMENT.md

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh bloody hell. I did look for something like development.md, totally missed the link in the main readme and then wondered why contributing.md was so slim. My fault 🤠


To set up your local development environment:

```bash
# Clone the repository
git clone https://github.com/open-telemetry/opentelemetry-php.git
cd opentelemetry-php

# Install dependencies
make install
```

To update dependencies:

```bash
make update
```

To run checks:

```bash
make all-checks
```


## Testing

To ensure your changes meet our project's standards, simply run:

```bash
RichardChukwu marked this conversation as resolved.
Show resolved Hide resolved
make all

```

To run tests against different PHP versions:

```bash
PHP_VERSION=8.3 make test
```

## Contributing Rules

- Follow [Clean Code PHP](https://github.com/jupeter/clean-code-php) principles.
- Ensure new features have appropriate test coverage.
- Run `make style` before submitting a PR.
- Include clear and concise documentation updates if needed.

Check for issues labeled [`good first issue`](https://github.com/open-telemetry/opentelemetry-php/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22) to start contributing.


## Further Help

Need help? Join our community:

Most of our communication is done on CNCF Slack in the channel:

- **Slack**: [otel-php](https://cloud-native.slack.com/archives/C01NFPCV44V)
- **GitHub Discussions**: [OpenTelemetry PHP Discussions](https://github.com/open-telemetry/opentelemetry-php/discussions)
- **Issues**: If you encounter a bug, [open an issue](https://github.com/open-telemetry/opentelemetry-php/issues)


## Troubleshooting Guide

### Common Issues & Fixes

#### 1. Missing PHP dependencies
**Error:** `Class 'SomeClass' not found`

**Fix:** Run:
```bash
make install
```

#### 2. Linting Errors
**Error:** `Files not formatted correctly`

**Fix:** Run:
```bash
make style
```

#### 3. Tests Failing Due to Missing Dependencies
**Error:** `Dependency missing`

**Fix:**
```bash
make update
make test
```


## Additional Information

### Code Coverage
We use [Codecov](https://about.codecov.io/) to track test coverage. You can generate a local coverage report using:

```bash
make test-coverage
```

### Generating API Documentation
To generate API docs:
```bash
make phpdoc
```

To preview locally:
```bash
make phpdoc-preview
```

### Dependency Validation
We use [Deptrac](https://github.com/qossmic/deptrac) for dependency validation:
```bash
make deptrac
```

Thank you for contributing! 🚀



## Maintainers
[@open-telemetry/php-maintainers](https://github.com/orgs/open-telemetry/teams/php-maintainers)

Expand Down Expand Up @@ -34,10 +201,6 @@ Find more information about the member role in the [community repository](https:

Find more about emeritus roles in the [community repository](https://github.com/open-telemetry/community/blob/main/community-membership.md#emeritus-maintainerapprovertriager)

## Communication
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for this to not be mentioned?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is mentioned actually in the guide, see under "Further Help" and the last information in the doc. @brettmc


Most of our communication is done on CNCF Slack in the channel [otel-php](https://cloud-native.slack.com/archives/C01NFPCV44V).
To sign up, create a CNCF Slack account [here](http://slack.cncf.io/)

Our meetings are held weekly on zoom on Wednesdays at 10:30am PST / 1:30pm EST.
A Google calendar invite with the included zoom link can be found [here](https://calendar.google.com/event?action=TEMPLATE&tmeid=N2VtZXZmYnVmbzZkYjZkbTYxdjZvYTdxN21fMjAyMDA5MTZUMTczMDAwWiBrYXJlbnlyeHVAbQ&tmsrc=google.com_b79e3e90j7bbsa2n2p5an5lf60%40group.calendar.google.com&scp=ALL)
Expand Down