Skip to content

Commit 419b93d

Browse files
Update contributing guide (#8110)
* Fix linting * chore: update CONTRIBUTING guide and root test script * feedback fixes --------- Co-authored-by: Carmen Popoviciu <[email protected]>
1 parent 82a8937 commit 419b93d

File tree

2 files changed

+91
-38
lines changed

2 files changed

+91
-38
lines changed

CONTRIBUTING.md

Lines changed: 90 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -30,30 +30,37 @@ Any contributions you make will be via [Pull Requests](https://docs.github.com/e
3030
- Ensure you have [created an account](https://docs.github.com/en/get-started/onboarding/getting-started-with-your-github-account) on GitHub.
3131
- [Create your own fork](https://docs.github.com/en/get-started/quickstart/fork-a-repo) of [this repository](https://github.com/cloudflare/workers-sdk).
3232
- Clone your fork to your local machine
33+
3334
```sh
34-
> git clone https://github.com/<your-github-username>/workers-sdk
35-
> cd workers-sdk
35+
git clone https://github.com/<your-github-username>/workers-sdk
36+
cd workers-sdk
3637
```
38+
3739
You can see that your fork is setup as the `origin` remote repository.
3840
Any changes you wish to make should be in a local branch that is then pushed to this origin remote.
41+
3942
```sh
40-
> git remote -v
43+
git remote -v
4144
origin https://github.com/<your-github-username>/workers-sdk (fetch)
4245
origin https://github.com/<your-github-username>/workers-sdk (push)
4346
```
47+
4448
- Add `cloudflare/workers-sdk` as the `upstream` remote repository.
49+
4550
```sh
46-
> git remote add upstream https://github.com/cloudflare/workers-sdk
47-
> git remote -v
51+
git remote add upstream https://github.com/cloudflare/workers-sdk
52+
git remote -v
4853
origin https://github.com/<your-github-username>/workers-sdk (fetch)
4954
origin https://github.com/<your-github-username>/workers-sdk (push)
5055
upstream https://github.com/cloudflare/workers-sdk (fetch)
5156
upstream https://github.com/cloudflare/workers-sdk (push)
5257
```
58+
5359
- You should regularly pull from the `main` branch of the `upstream` repository to keep up to date with the latest changes to the project.
60+
5461
```sh
55-
> git switch main
56-
> git pull upstream main
62+
git switch main
63+
git pull upstream main
5764
From https://github.com/cloudflare/workers-sdk
5865
* branch main -> FETCH_HEAD
5966
Already up to date.
@@ -98,31 +105,36 @@ While each workspace has its own dependencies, you install the dependencies usin
98105
> If you haven't used `pnpm` before, you can install it with `npm install -g pnpm`
99106
100107
- Install all the dependencies
108+
101109
```sh
102-
> cd workers-sdk
103-
> pnpm install
110+
cd workers-sdk
111+
pnpm install
104112
```
105113

106114
## Building and running
107115

108116
Workspaces in this project are mostly written in [TypeScript](https://www.typescriptlang.org/) and compiled, by [esbuild](https://github.com/evanw/esbuild), into JavaScript bundles for distribution.
109117

110118
- Run a distributable for a specific workspace (e.g. wrangler)
119+
111120
```sh
112-
> pnpm run --filter wrangler start
121+
pnpm run --filter wrangler start
113122
```
123+
114124
- Build a distributable for a specific workspace (e.g. wrangler)
125+
115126
```sh
116-
> pnpm run build --filter wrangler
127+
pnpm run build --filter wrangler
117128
```
118129

119130
## Checking the code
120131

121132
The code in the repository is checked for type checking, formatting, linting and testing errors.
122133

123134
- Run all checks in all the workspaces
135+
124136
```sh
125-
> pnpm run check
137+
pnpm run check
126138
```
127139

128140
When doing normal development, you may want to run these checks individually.
@@ -132,9 +144,11 @@ When doing normal development, you may want to run these checks individually.
132144
The code is checked for type errors by [TypeScript](https://www.typescriptlang.org/).
133145

134146
- Type check all the code in the repository
147+
135148
```sh
136-
> pnpm run check:type
149+
pnpm run check:type
137150
```
151+
138152
- VS Code will also run type-checking while editing source code, providing immediate feedback.
139153

140154
#### Changing TypeScript Version in VS Code's Command Palette
@@ -149,7 +163,7 @@ For TypeScript to work properly in the Monorepo the version used in VSCode must
149163

150164
4. A submenu will appear with a list of available TypeScript versions. Choose the desired version you want to use for this project. If you have multiple versions installed, they will be listed here.
151165

152-
- Selecting "Use Workspace Version" will use the version of TypeScript installed in the project's `node_modules` directory.
166+
- Selecting "Use Workspace Version" will use the version of TypeScript installed in the project's `node_modules` directory.
153167

154168
5. After selecting the TypeScript version, VSCode will reload the workspace using the chosen version.
155169

@@ -161,41 +175,68 @@ Remember that this change is specific to the current project and will not affect
161175
The code is checked for linting errors by [ESLint](https://eslint.org/).
162176

163177
- Run the linting checks
178+
164179
```sh
165-
> pnpm run check:lint
180+
pnpm run check:lint
166181
```
182+
167183
- The repository has a recommended VS Code plugin to run ESLint checks while editing source code, providing immediate feedback.
168184

169185
### Formatting
170186

171187
The code is checked for formatting errors by [Prettier](https://prettier.io/).
172188

173189
- Run the formatting checks
190+
174191
```sh
175-
> pnpm run check:format
192+
pnpm run check:format
176193
```
194+
177195
- The repository has a recommended VS Code plugin to run Prettier checks, and to automatically format using Prettier, while editing source code, providing immediate feedback
178196
- Use the following command to run prettier on the codebase
197+
179198
```sh
180-
> pnpm run prettify
199+
pnpm run prettify
181200
```
182201

183202
### Testing
184203

185204
Tests in a workspace are executed, by [Vitest](https://vitest.dev/), which is configured to automatically compile and bundle the TypeScript before running the tests.
186205

206+
- If you have recently rebased on `main` then make sure you have installed any new dependencies
207+
208+
```sh
209+
pnpm i
210+
```
211+
187212
- Run the tests for all the workspaces
213+
188214
```sh
189-
> pnpm run test
215+
pnpm run test
190216
```
217+
218+
:::note
219+
Cloudflare employees may need to turn off WARP for the first time they run the Miniflare tests so that it can request and cache the CF properties without getting the following error.
220+
221+
```plain
222+
failed: TLS peer's certificate is not trusted; reason = self signed certificate in certificate chain
223+
```
224+
225+
After this request is cached you can run tests with WARP turned on, no problem.
226+
:::
227+
191228
- Run the tests for a specific workspace (e.g. wrangler)
229+
192230
```sh
193-
> pnpm run test --filter wrangler
231+
pnpm run test --filter wrangler
194232
```
233+
195234
- Watch the files in a specific workspace (e.g. wrangler), and run the tests when anything changes
235+
196236
```sh
197-
> pnpm run --filter wrangler test:watch
237+
pnpm run --filter wrangler test:watch
198238
```
239+
199240
This will also run all the tests in a single process (rather than in parallel shards) and will increase the test-timeout to 50 seconds, which is helpful when debugging.
200241

201242
## Steps For Making Changes
@@ -204,25 +245,34 @@ Every change you make should be stored in a [git commit](https://github.com/git-
204245
Changes should be committed to a new local branch, which then gets pushed to your fork of the repository on GitHub.
205246

206247
- Ensure your `main` branch is up to date
248+
207249
```sh
208-
> git switch main
209-
> git pull upstream main
250+
git switch main
251+
git pull upstream main
210252
```
253+
211254
- Create a new branch, based off the `main` branch
255+
212256
```sh
213-
> git checkout -b <new-branch-name> main
257+
git checkout -b <new-branch-name> main
214258
```
259+
215260
- Stage files to include in a commit
261+
216262
- Use [VS Code](https://code.visualstudio.com/docs/editor/versioncontrol#_git-support)
217263
- Or add and commit files via the command line
264+
218265
```sh
219-
> git add <paths-to-changes-files>
220-
> git commit
266+
git add <paths-to-changes-files>
267+
git commit
221268
```
269+
222270
- Push changes to your fork
271+
223272
```sh
224273
git push -u origin <new-branch-name>
225274
```
275+
226276
- Once you are happy with your changes, create a Pull Request on GitHub
227277
- The format for Pull Request titles is `[package name] description`, where the package name should indicate which package of the `workers-sdk` monorepo your PR pertains to (e.g. `wrangler`/`pages-shared`/`chrome-devtools-patches`), and the description should be a succinct summary of the change you're making.
228278
- GitHub will insert a template for the body of your Pull Request—it's important to carefully fill out all the fields, giving as much detail as possible to reviewers.
@@ -232,9 +282,9 @@ Changes should be committed to a new local branch, which then gets pushed to you
232282
Making sure your branch follows our recommendations for git will help ensure your PR is reviewed & released as quickly as possible:
233283

234284
- When opening a PR (before the first review), try and make sure your git commit history is clean, and clearly describes the changes you want to make.
235-
- For instance, here's an example of a PR where the commit history is quite messy, and doesn't help reviewers: https://github.com/cloudflare/workers-sdk/pull/2409/commits
236-
- And here's an example of where this has been done well: https://github.com/cloudflare/workers-sdk/pull/4795/commits
237-
- Once your PR has been reviewed, when addressing feedback try not to modify already reviewed commits with force pushes. This slows down the review process and makes it hard to keep track of what changes have been made. Instead, add aditional commits to your PR to address any feedback (`git commit --fixup` is a helpful tool here).
285+
- For instance, here's an example of a PR where the commit history is quite messy, and doesn't help reviewers: <https://github.com/cloudflare/workers-sdk/pull/2409/commits>
286+
- And here's an example of where this has been done well: <https://github.com/cloudflare/workers-sdk/pull/4795/commits>
287+
- Once your PR has been reviewed, when addressing feedback try not to modify already reviewed commits with force pushes. This slows down the review process and makes it hard to keep track of what changes have been made. Instead, add additional commits to your PR to address any feedback (`git commit --fixup` is a helpful tool here).
238288
- When merging your PR into `main`, `workers-sdk` enforces squash merges. As such, please try and make sure that the commit message associated with the merge clearly describes the entire change your PR makes.
239289

240290
## PR Review
@@ -243,7 +293,7 @@ PR review is a critical and required step in the process for landing changes. Th
243293

244294
## PR Previews
245295

246-
Every PR will have an associated pre-release build for all releaseable packages within the repository, powered by our [prerelease registry](packages/prerelease-registry). You can find links to prereleases for each package in a comment automatically posted by GitHub Actions on each opened PR ([for example](https://github.com/cloudflare/workers-sdk/pull/7172#issuecomment-2457244715)).
296+
Every PR will have an associated pre-release build for all releasable packages within the repository, powered by our [prerelease registry](packages/prerelease-registry). You can find links to prereleases for each package in a comment automatically posted by GitHub Actions on each opened PR ([for example](https://github.com/cloudflare/workers-sdk/pull/7172#issuecomment-2457244715)).
247297

248298
It's also possible to generate preview builds for the applications in the repository. These aren't generated automatically because they're pretty slow CI jobs, but you can trigger preview builds by adding one of the following labels to your PR:
249299

@@ -257,8 +307,8 @@ Once built, you can find the preview link for these applications in the [Deploy
257307

258308
Every PR should include tests for the functionality that's being added. Most changes will be to [Wrangler](packages/wrangler/src/__tests__) (using Vitest), [Miniflare](packages/miniflare/test) (using Ava), or [C3](packages/create-cloudflare/src/__tests__) (using Vitest), and should include unit tests within the testing harness of those packages. For documentation on how these testing frameworks work, see:
259309

260-
- Vitest: https://vitest.dev/guide
261-
- Ava: https://github.com/avajs/ava?tab=readme-ov-file#documentation
310+
- Vitest: <https://vitest.dev/guide>
311+
- Ava: <https://github.com/avajs/ava?tab=readme-ov-file#documentation>
262312

263313
If your PR includes functionality that's difficult to unit test, you can add a fixture test by creating a new package in the `fixtures/` folder. This allows for adding a test that requires a specific filesystem or worker setup (for instance, `fixtures/no-bundle-import` tests the interaction of Wrangler with a specific set of JS, WASM, text, and binary modules on the filesystem). When adding a fixture test, include a `vitest.config.mts` file within the new package, which will ensure it's run as part of the `workers-sdk` CI. You should merge your own configuration with the default config from the root of the repo.
264314

@@ -289,13 +339,13 @@ A summary of this repositories actions can be found [here](.github/workflows/REA
289339
To run the e2e tests locally, you'll need a Cloudflare API Token and run:
290340

291341
```sh
292-
$ WRANGLER="node ~/path/to/workers-sdk/packages/wrangler/wrangler-dist/cli.js" CLOUDFLARE_ACCOUNT_ID=$CLOUDFLARE_TESTING_ACCOUNT_ID CLOUDFLARE_API_TOKEN=$CLOUDFLARE_TESTING_API_TOKEN pnpm run test:e2e
342+
WRANGLER="node ~/path/to/workers-sdk/packages/wrangler/wrangler-dist/cli.js" CLOUDFLARE_ACCOUNT_ID=$CLOUDFLARE_TESTING_ACCOUNT_ID CLOUDFLARE_API_TOKEN=$CLOUDFLARE_TESTING_API_TOKEN pnpm run test:e2e
293343
```
294344

295345
You may optionally want to append a filename pattern to limit which e2e tests are run. Also you may want to set `--bail=n` to limit the number of fails tests to show the error before the rest of the tests finish running and to limit the noise in that output:
296346

297347
```sh
298-
$ WRANGLER="node ~/path/to/workers-sdk/packages/wrangler/wrangler-dist/cli.js" CLOUDFLARE_ACCOUNT_ID=$CLOUDFLARE_TESTING_ACCOUNT_ID CLOUDFLARE_API_TOKEN=$CLOUDFLARE_TESTING_API_TOKEN pnpm run test:e2e [file-pattern] --bail=1
348+
WRANGLER="node ~/path/to/workers-sdk/packages/wrangler/wrangler-dist/cli.js" CLOUDFLARE_ACCOUNT_ID=$CLOUDFLARE_TESTING_ACCOUNT_ID CLOUDFLARE_API_TOKEN=$CLOUDFLARE_TESTING_API_TOKEN pnpm run test:e2e [file-pattern] --bail=1
299349
```
300350

301351
### Creating an API Token
@@ -322,14 +372,17 @@ Every non-trivial change to the project - those that should appear in the change
322372
We use the [`changesets`](https://github.com/changesets/changesets/blob/main/README.md) tool for creating changesets, publishing versions and updating the changelog.
323373

324374
- Create a changeset for the current change.
375+
325376
```sh
326-
> pnpm changeset
377+
pnpm changeset
327378
```
379+
328380
- Select which workspaces are affected by the change and whether the version requires a major, minor or patch release.
329381
- Update the generated changeset with a description of the change.
330382
- Include the generate changeset in the current commit.
383+
331384
```sh
332-
> git add ./changeset/*.md
385+
git add ./changeset/*.md
333386
```
334387

335388
### Changeset message format
@@ -338,7 +391,7 @@ Each changeset is a file that describes the change being merged. This file is us
338391

339392
To help maintain consistency in the changelog, changesets should have the following format:
340393

341-
```
394+
```plain
342395
<TITLE>
343396
344397
<BODY>
@@ -353,7 +406,7 @@ The generated changeset file will contain the package name and type of change (e
353406

354407
Here's an example of a `patch` to the `wrangler` package:
355408

356-
```
409+
```plain
357410
---
358411
"wrangler": patch
359412
---

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
"dev": "dotenv -- turbo dev",
2121
"fix": "dotenv -- turbo check:lint -- --fix && pnpm run prettify",
2222
"prettify": "prettier . --write --ignore-unknown",
23-
"test": "vitest run --no-file-parallelism && dotenv -- turbo test --filter=wrangler --filter=miniflare --filter=kv-asset-handler --filter=@cloudflare/vitest-pool-workers --filter=@cloudflare/vitest-pool-workers-examples",
23+
"test": "dotenv -- turbo test",
2424
"test:ci": "dotenv -- turbo test:ci",
2525
"test:e2e": "dotenv -- turbo test:e2e",
2626
"test:e2e:wrangler": "node -r esbuild-register tools/e2e/runIndividualE2EFiles.ts",

0 commit comments

Comments
 (0)