You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Copy file name to clipboardExpand all lines: CONTRIBUTING.md
+70-37Lines changed: 70 additions & 37 deletions
Original file line number
Diff line number
Diff line change
@@ -6,17 +6,18 @@
6
6
7
7
# Contributing
8
8
9
-
We welcome suggested improvements and bug fixes to the `@sentry/*` family of packages, in the form of pull requests on [`GitHub`](https://github.com/getsentry/sentry-javascript). The guide below will help you get started, but if you have further questions, please feel free to reach out on [Discord](https://discord.gg/Ww9hbqr).
10
-
9
+
We welcome suggested improvements and bug fixes to the `@sentry/*` family of packages, in the form of pull requests on
10
+
[`GitHub`](https://github.com/getsentry/sentry-javascript). The guide below will help you get started, but if you have
11
+
further questions, please feel free to reach out on [Discord](https://discord.gg/Ww9hbqr).
11
12
12
13
## Setting up an Environment
13
14
14
15
To run the test suite and our code linter, node.js and yarn are required.
`sentry-javascript` is a monorepo containing several packages, and we use `lerna` to manage them. To get started, install all dependencies, and then perform an initial build, so TypeScript can read all of the linked type definitions.
19
+
`sentry-javascript` is a monorepo containing several packages, and we use `lerna` to manage them. To get started,
20
+
install all dependencies, and then perform an initial build, so TypeScript can read all of the linked type definitions.
20
21
21
22
```
22
23
$ yarn
@@ -27,58 +28,78 @@ With that, the repo is fully set up and you are ready to run all commands.
27
28
28
29
## Building Packages
29
30
30
-
Since we are using [`TypeScript`](https://www.typescriptlang.org/), you need to transpile the code to JavaScript to be able to use it. From the top level of the repo, there are three commands available:
31
+
Since we are using [`TypeScript`](https://www.typescriptlang.org/), you need to transpile the code to JavaScript to be
32
+
able to use it. From the top level of the repo, there are three commands available:
31
33
32
34
-`yarn build:dev`, which runs a one-time build of ES5 and ES6 versions of every package
33
-
-`yarn build:dev:filter <name of npm package>`, which runs `yarn build:dev` only in projects relevant to the given package (so, for example, running `yarn build:dev:filter @sentry/react` will build the `react` package, all of its dependencies (`utils`, `core`, `browser`, etc), and all packages which depend on it (currently `gatsby` and `nextjs`))
35
+
-`yarn build:dev:filter <name of npm package>`, which runs `yarn build:dev` only in projects relevant to the given
36
+
package (so, for example, running `yarn build:dev:filter @sentry/react` will build the `react` package, all of its
37
+
dependencies (`utils`, `core`, `browser`, etc), and all packages which depend on it (currently `gatsby` and `nextjs`))
34
38
-`yarn build:dev:watch`, which runs `yarn build:dev` in watch mode (recommended)
35
39
36
-
37
40
## Testing SDK Packages Locally
38
41
39
42
To test local versions of SDK packages, for instance in test projects, you have a couple of options:
40
43
41
-
* Use [`yarn link`](https://classic.yarnpkg.com/lang/en/docs/cli/link/) to symlink your package to the test project.
42
-
* Use [`yalc` to install SDK packages](./docs/using-yalc.md) as if they were already published.
43
-
* Run `build:tarball` in the repo and `yarn add ./path/to/tarball.tgz` in the project.
44
+
- Use [`yarn link`](https://classic.yarnpkg.com/lang/en/docs/cli/link/) to symlink your package to the test project.
45
+
- Use [`yalc` to install SDK packages](./docs/using-yalc.md) as if they were already published.
46
+
- Run `build:tarball` in the repo and `yarn add ./path/to/tarball.tgz` in the project.
44
47
45
48
## Adding Tests
46
49
47
50
**Any nontrivial fixes/features should include tests.** You'll find a `test` folder in each package.
48
51
49
-
Note that _for the `browser` package only_, if you add a new file to the [integration test suite](https://github.com/getsentry/sentry-javascript/tree/master/packages/browser/test/integration/suites), you also need to add it to [the list in `shell.js`](https://github.com/getsentry/sentry-javascript/blob/b74e199254147fd984e7bb1ea24193aee70afa74/packages/browser/test/integration/suites/shell.js#L25) as well. Adding tests to existing files will work out of the box in all packages.
52
+
Note that _for the `browser` package only_, if you add a new file to the
53
+
[integration test suite](https://github.com/getsentry/sentry-javascript/tree/master/packages/browser/test/integration/suites),
54
+
you also need to add it to
55
+
[the list in `shell.js`](https://github.com/getsentry/sentry-javascript/blob/b74e199254147fd984e7bb1ea24193aee70afa74/packages/browser/test/integration/suites/shell.js#L25)
56
+
as well. Adding tests to existing files will work out of the box in all packages.
50
57
51
58
## Running Tests
52
59
53
-
Running tests works the same way as building - running `yarn test` at the project root will run tests for all packages, and running `yarn test` in a specific package will run tests for that package. There are also commands to run subsets of the tests in each location. Check out the `scripts` entry of the corresponding `package.json` for details.
60
+
Running tests works the same way as building - running `yarn test` at the project root will run tests for all packages,
61
+
and running `yarn test` in a specific package will run tests for that package. There are also commands to run subsets of
62
+
the tests in each location. Check out the `scripts` entry of the corresponding `package.json` for details.
54
63
55
64
Note: you must run `yarn build` before `yarn test` will work.
56
65
57
66
## Debugging Tests
58
67
59
68
If you run into trouble writing tests and need to debug one of them, you can do so using VSCode's debugger.
60
69
61
-
0. If you don't already have it installed, install the Tasks Shell Input extension, which you'll find in the Extensions tab in the sidebar as one of the recommended workspace extensions.
70
+
0. If you don't already have it installed, install the Tasks Shell Input extension, which you'll find in the Extensions
71
+
tab in the sidebar as one of the recommended workspace extensions.
62
72
63
73
1. Place breakpoints or `debugger` statements in the test or the underlying code wherever you'd like `jest` to pause.
64
74
2. Open the file containing the test in question, and make sure its tab is active (so you can see the file's contents).
65
75
3. Switch to the debugger in the sidebar and choose `Debug unit tests - just open file` from the dropdown.
66
76
4. Click the green "play" button to run the tests in the open file in watch mode.
67
77
68
-
Pro tip: If any of your breakpoints are in code run by multiple tests, and you run the whole test file, you'll land on those breakpoints over and over again, in the middle of tests you don't care about. To avoid this, replace the test's initial `it` or `test` with `it.only` or `test.only`. That way, when you hit a breakpoint, you'll know you got there are part of the buggy test.
78
+
Pro tip: If any of your breakpoints are in code run by multiple tests, and you run the whole test file, you'll land on
79
+
those breakpoints over and over again, in the middle of tests you don't care about. To avoid this, replace the test's
80
+
initial `it` or `test` with `it.only` or `test.only`. That way, when you hit a breakpoint, you'll know you got there are
81
+
part of the buggy test.
69
82
70
83
## Debug Build Flags
71
84
72
85
Throughout the codebase, you will find a `__DEBUG_BUILD__` constant. This flag serves two purposes:
73
86
74
-
1. It enables us to remove debug code from our minified CDN bundles during build, by replacing the flag with `false` before tree-shaking occurs.
75
-
2. It enables users to remove Sentry debug code from their production bundles during their own build. When we build our npm packages, we replace the flag with `(typeof __SENTRY_DEBUG__ === 'undefined' || __SENTRY_DEBUG__)`. If the user does nothing, this evaluates to `true` and logging is included. But if the user runs their own replacement during build (again replacing the flag with `false`), the build will tree-shake the logging away, just as our bundle builds do.
87
+
1. It enables us to remove debug code from our minified CDN bundles during build, by replacing the flag with `false`
88
+
before tree-shaking occurs.
89
+
2. It enables users to remove Sentry debug code from their production bundles during their own build. When we build our
90
+
npm packages, we replace the flag with `(typeof __SENTRY_DEBUG__ === 'undefined' || __SENTRY_DEBUG__)`. If the user
91
+
does nothing, this evaluates to `true` and logging is included. But if the user runs their own replacement during
92
+
build (again replacing the flag with `false`), the build will tree-shake the logging away, just as our bundle builds
93
+
do.
76
94
77
-
Note that the replacement flag, `__SENTRY_DEBUG__`, is different from the original flag . This is necessary because the replacement plugin runs twice, at two different stages of the build, and we don't want to run a replacement on our replacement (as would happen if we reused `__DEBUG_BUILD__`).
95
+
Note that the replacement flag, `__SENTRY_DEBUG__`, is different from the original flag . This is necessary because the
96
+
replacement plugin runs twice, at two different stages of the build, and we don't want to run a replacement on our
97
+
replacement (as would happen if we reused `__DEBUG_BUILD__`).
78
98
79
99
## Linting
80
100
81
-
Similar to building and testing, linting can be done in the project root or in individual packages by calling `yarn lint`.
101
+
Similar to building and testing, linting can be done in the project root or in individual packages by calling
102
+
`yarn lint`.
82
103
83
104
Note: you must run `yarn build` before `yarn lint` will work.
84
105
@@ -89,24 +110,34 @@ When contributing to the codebase, please note:
89
110
- Make sure to follow the [Commit, Issue & PR guidelines](#commit-issue--pr-guidelines)
90
111
- Non-trivial PRs will not be accepted without tests (see above).
91
112
- Please do not bump version numbers yourself.
92
-
-[`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).
113
+
-[`raven-js`](https://github.com/getsentry/sentry-javascript/tree/3.x/packages/raven-js) and
114
+
[`raven-node`](https://github.com/getsentry/sentry-javascript/tree/3.x/packages/raven-node) are deprecated, and only
115
+
bug and security fix PRs will be accepted targeting the
116
+
[3.x branch](https://github.com/getsentry/sentry-javascript/tree/3.x). Any new features and improvements should be to
117
+
our new SDKs (`browser`, `node`, and framework-specific packages like `react` and `nextjs`) and the packages which
118
+
support them (`core`, `utils`, `integrations`, and the like).
93
119
94
120
## PR reviews
95
121
96
-
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:
122
+
For feedback in PRs, we use the [LOGAF scale](https://blog.danlew.net/2020/04/15/the-logaf-scale/) to specify how
123
+
important a comment is:
97
124
98
-
*`l`: low - nitpick. You may address this comment, but you don't have to.
99
-
*`m`: medium - normal comment. Worth addressing and fixing.
100
-
*`h`: high - Very important. We must not merge this PR without addressing this issue.
125
+
-`l`: low - nitpick. You may address this comment, but you don't have to.
126
+
-`m`: medium - normal comment. Worth addressing and fixing.
127
+
-`h`: high - Very important. We must not merge this PR without addressing this issue.
101
128
102
-
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.
129
+
You only need one approval from a maintainer to be able to merge. For some PRs, asking specific or multiple people for
130
+
review might be adequate.
103
131
104
132
Our different types of reviews:
105
133
106
-
1.**LGTM without any comments.** You can merge immediately.
107
-
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.
108
-
3.**Only comments.** You must address all the comments and need another review until you merge.
109
-
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.
134
+
1.**LGTM without any comments.** You can merge immediately.
135
+
2.**LGTM with low and medium comments.** The reviewer trusts you to resolve these comments yourself, and you don't need
136
+
to wait for another approval.
137
+
3.**Only comments.** You must address all the comments and need another review until you merge.
138
+
4.**Request changes.** Only use if something critical is in the PR that absolutely must be addressed. We usually use
139
+
`h` comments for that. When someone requests changes, the same person must approve the changes to allow merging. Use
140
+
this sparingly.
110
141
111
142
## Commit, Issue & PR guidelines
112
143
@@ -116,7 +147,7 @@ For commit messages, we use the format:
116
147
117
148
```
118
149
<type>(<scope>): <subject> (<github-id>)
119
-
````
150
+
```
120
151
121
152
For example: `feat(core): Set custom transaction source for event processors (#5722)`.
122
153
@@ -126,17 +157,19 @@ The Github-ID can be left out until the PR is merged.
126
157
127
158
### Issues
128
159
129
-
Issues should at least be categorized by package, for example `package: Node`.
130
-
Additional labels for categorization can be added, and the Sentry SDK team may also add further labels as needed.
160
+
Issues should at least be categorized by package, for example `package: Node`. Additional labels for categorization can
161
+
be added, and the Sentry SDK team may also add further labels as needed.
131
162
132
163
### Pull Requests (PRs)
133
164
134
-
PRs are merged via `Squash and merge`.
135
-
This means that all commits on the branch will be squashed into a single commit, and committed as such onto master.
165
+
PRs are merged via `Squash and merge`. This means that all commits on the branch will be squashed into a single commit,
166
+
and committed as such onto master.
136
167
137
-
* The PR name can generally follow the commit name (e.g. `feat(core): Set custom transaction source for event processors`)
138
-
* Make sure to rebase the branch on `master` before squashing it
139
-
* Make sure to update the commit message of the squashed branch to follow the commit guidelines - including the PR number
168
+
- The PR name can generally follow the commit name (e.g.
169
+
`feat(core): Set custom transaction source for event processors`)
170
+
- Make sure to rebase the branch on `master` before squashing it
171
+
- Make sure to update the commit message of the squashed branch to follow the commit guidelines - including the PR
0 commit comments