From f2b941ae120e78c9ec2c4c0d805842477036c518 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 18 Oct 2022 12:37:33 +0200 Subject: [PATCH] chore: Add logaf guide to CONTRIBUTING docs --- CONTRIBUTING.md | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e5573d07a8db..33ce9f323ee9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -80,7 +80,24 @@ When contributing to the codebase, please note: - 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`, `hub`, `integrations`, and the like). +- [`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). + +## PR reviews + +For feedback in PRs, we use the [LOGAF scale](https://blog.danlew.net/2020/04/15/the-logaf-scale/) to specify how important a comment is: + +* `l`: low - nitpick. You may address this comment, but you don't have to. +* `m`: medium - normal comment. Worth addressing and fixing. +* `h`: high - Very important. We must not merge this PR without addressing this issue. + +You only need one approval from a maintainer to be able to merge. For some PRs, asking specific or multiple people for review might be adequate. + +Our different types of reviews: + + 1. **LGTM without any comments.** You can merge immediately. + 2. **LGTM with low and medium comments.** The reviewer trusts you to resolve these comments yourself, and you don't need to wait for another approval. + 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. ## Publishing a Release