Skip to content

chore(docs): Add commit, issue & PR guidelines #6022

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 24, 2022
Merged

chore(docs): Add commit, issue & PR guidelines #6022

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 31 additions & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ Note: you must run `yarn build` before `yarn lint` will work.

When contributing to the codebase, please note:

- Make sure to follow the [Commit, Issue & PR guidelines](#commit-issue--pr-guidelines)
- Non-trivial PRs will not be accepted without tests (see above).
- Please do not bump version numbers yourself.
- [`raven-js`](https://github.com/getsentry/sentry-javascript/tree/3.x/packages/raven-js) and [`raven-node`](https://github.com/getsentry/sentry-javascript/tree/3.x/packages/raven-node) are deprecated, and only bug and security fix PRs will be accepted targeting the [3.x branch](https://github.com/getsentry/sentry-javascript/tree/3.x). Any new features and improvements should be to our new SDKs (`browser`, `node`, and framework-specific packages like `react` and `nextjs`) and the packages which support them (`core`, `utils`, `integrations`, and the like).
Expand All @@ -99,6 +100,36 @@ Our different types of reviews:
3. **Only comments.** You must address all the comments and need another review until you merge.
4. **Request changes.** Only use if something critical is in the PR that absolutely must be addressed. We usually use `h` comments for that. When someone requests changes, the same person must approve the changes to allow merging. Use this sparingly.

## Commit, Issue & PR guidelines

### Commits

For commit messages, we use the format:

```
<type>(<scope>): <subject> (<github-id>)
````

For example: `feat(core): Set custom transaction source for event processors (#5722)`.
Copy link
Member

Choose a reason for hiding this comment

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

l: Should we point out that this format only strictly has to apply to the squashed commit that we merge into master?

Asking because what I'm usually doing is that in the first commit of my branch, I use the <type>(<scope>): <subject> format but without any github id. For follow-up commits I usually don't stick strictly to the convention. Not saying that this is the best way to go but IMO, since we squash and get the chance to rewrite the message before merging, this is the only message that actually matters if that makes sense...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, good point, I also thought about this a bit.

Maybe add a part like:

The Github-ID can be left out until the PR is merged

Does that make sense?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good!


See [commit message format](https://develop.sentry.dev/commit-messages/#commit-message-format) for details.

The Github-ID can be left out until the PR is merged.

### Issues

Issues should at least be categorized by package, for example `package: Node`.
Additional labels for categorization can be added, and the Sentry SDK team may also add further labels as needed.

### Pull Requests (PRs)

PRs are merged via `Squash and merge`.
This means that all commits on the branch will be squashed into a single commit, and commited as such onto master.

* The PR name can generally follow the commit name (e.g. `feat(core): Set custom transaction source for event processors`)
* Make sure to rebase the branch on `master` before squashing it
* Make sure to update the commit message of the squashed branch to follow the commit guidelines - including the PR number

## Publishing a Release

_These steps are only relevant to Sentry employees when preparing and publishing a new SDK release._
Expand Down