Skip to content

Commit 275a693

Browse files
authored
Updated README and CONTRIBUTING (expo#5603)
* Updated contributing * Added contributing guide * Updated docs from feedback * Updated contributing * Update download-dependencies.sh * Update CONTRIBUTING.md * Update CONTRIBUTING.md * Update CONTRIBUTING.md * Update CONTRIBUTING.md * Moved code standards to a new guide * Added TOCs * Fixed TOCs * Update CONTRIBUTING.md * Updated setup scripts * Updated setup scripts * Update CONTRIBUTING.md * Update CONTRIBUTING.md Co-Authored-By: James Ide <[email protected]> * Update CONTRIBUTING.md Co-Authored-By: James Ide <[email protected]> * Update CONTRIBUTING.md * Update CONTRIBUTING.md * Update README.md
1 parent 02b6d9e commit 275a693

14 files changed

+569
-245
lines changed

CONTRIBUTING.md

Lines changed: 109 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,131 @@
11
# Contributing to Expo
22

3-
We ask pull requests to be coherent and maintainable, and require code review by the Expo team.
3+
- [📦 Download and Setup](#-download-and-setup)
4+
- [✍️ Editing Packages](#-editing-packages)
5+
- [Extra Credit](#extra-credit)
6+
- [⏱ Testing Your Changes](#-testing-your-changes)
7+
- [✅ Unit Testing](#-unit-testing)
8+
- [🏁 E2E Testing](#-e2e-testing)
9+
- [📚 Updating Documentation](#-updating-documentation)
10+
- [📝 Writing a Commit Message](#-writing-a-commit-message)
11+
- [🔎 Before Submitting](#-before-submitting)
12+
- [Extra Credit](#extra-credit-1)
413

5-
## Code reviews
14+
Thanks so much for coming to help! Currently we review PRs for `packages/`, `docs/`, `templates/`, `guides/`, `apps/`, and markdown files. Because the native clients (`ios/`, `android/`) are difficult to setup and require API tokens, not much progress can be made externally (but you can always try!). We've moved most of the fun code out of the client anyways to support the **bare-workflow**; this means that you'll do the majority of your native testing in a bare Expo project.
615

7-
The Expo team reviews all PRs and makes the judgement call on whether to accept them. An Expo team member will look at each PR and assign it to the appropriate reviewer for an in-depth review or request changes.
16+
As you might imagine web code is very easy to test and contribute to, so that's all on the table! You may find that some of the web features you're looking for are actually in the [expo-cli repo](https://github.com/expo/expo-cli).
817

9-
Writing a maintainable PR as described above is the best way to get it reviewed timely and potentially accepted. The easier to review and maintain the code, the more likely it will be accepted.
18+
## 📦 Download and Setup
1019

11-
## Updating the changelog
20+
1. [Fork](https://help.github.com/articles/fork-a-repo/) this repository to your own GitHub account and then [clone](https://help.github.com/articles/cloning-a-repository/) it to your local device. (`git remote add upstream [email protected]:expo/expo.git` 😉)
21+
2. Run the setup script with: `npm run setup:native` (if you just want to contribute to the docs, you can run `npm run setup:docs`). This command does the following for you:
1222

13-
Add a short, one-line description of the change to [CHANGELOG.md](/CHANGELOG.md), under the section appropriate for the change. This is especially helpful for breaking changes.
23+
<!-- TODO(Bacon): Split this into 2 scripts so people can contribute to docs without installing React Native -->
1424

15-
## Writing a commit message
25+
- Downloads submodules (like `react-native`) with `git submodule update --init`
26+
- Fetches files with [_git lfs_](https://git-lfs.github.com), which we use for big native libraries like Google Mobile Vision. Note: you must have `git lfs` already installed.
27+
- Ensures Yarn is installed
28+
- Ensures your computer is set up for React Native (will install the Android NDK if it's not present)
29+
- Downloads the Node packages (`yarn install`)
1630

17-
Commit messages should include a title, summary, and test plan.
31+
3. Navigate to the bare sandbox project `cd apps/bare-expo`
32+
4. Run the project on any platform (maybe start with web; it's the fastest! 😁)
1833

19-
Write the title in the imperative mood and prefix it with a tag that describes the affected code, like `[android]` or `[video]`, and makes it easier to read through the commit log.
34+
- Web: `yarn web`
35+
- iOS: `yarn ios`
36+
- Android: `yarn android`
2037

21-
In the summary, explain the motivation behind the commit ("why") and the approach it takes ("how"). Note things that aren't communicated by the code or require more context to infer.
38+
> If this didn't work for you as described, please [open an issue.](https://github.com/expo/expo/issues/new/choose)
2239
23-
Use the test plan to communicate how to verify the code actually works and to help others in the future create their test plans for the same area of the codebase. Read the Expo guide on [Git and Code Reviews](/guides/Git%20and%20Code%20Reviews.md) for more guidance on PRs and test plans.
40+
## ✍️ Editing Packages
2441

25-
This post called ["How to Write a Git Commit Message"](https://chris.beams.io/posts/git-commit/) has a lot of good guidance, too.
42+
All of our packages (including Foundation Unimodules) can be found in the `packages/` directory. These packages are automatically linked to the projects in the `apps/` directory (meaning any iOS, Android, web, or API changes can be tested from `apps/bare-expo/`). `bare-expo` is a bare workflow Expo app that acts as a runner for the other projects in the `apps/` directory.
2643

27-
## Guidance
44+
- `native-component-list`: This is where you can write demos or tests that require physical interaction (a good playground for testing).
45+
- `test-suite`: You can write your E2E tests in here. When pushed to the remote, CI will run this project with Device Farm for Android, Detox for iOS, and Puppeteer for web!
2846

29-
### On coherent pull requests
47+
All modules should adhere to the style guides which can be found here:
3048

31-
Each PR should correspond to one idea and implement it coherently. This idea may be a feature that spans several parts of the codebase. For example, changing an API may include changes to the Android, iOS, and web implementations, the JavaScript SDK, and the docs for the API.
49+
- [Guide to Unimodule Development](guides/Expo%20Universal%20Module%20Infrastructure.md)
50+
- [Expo JS Style Guide](guides/Expo%20JavaScript%20Style%20Guide.md) (also mostly applies to TypeScript)
3251

33-
Generally, each PR should contain one commit that is amended as you address code review feedback. Each commit should be meaningful and make sense on its own. Similarly, it should be easy to revert each commit. This keeps the commit history easier to read when people are working on this code or searching for a commit that could have broken something.
52+
1. Navigate to a package you want to edit. Ex: `cd packages/expo-constants`
53+
2. Start the TypeScript build in watch mode: `yarn build`
54+
3. Edit code in that package's `src/` directory
55+
4. Play with your changes on a simulator or device through `bare-expo`:
56+
- Add or modify a file named after the API you're working on. Ex: `apps/test-suite/tests/Constants.js`
57+
- Run the code from the `bare-expo` project with `yarn <android | ios | web>` or test the code with `yarn test:<ios | web>`
58+
5. You can edit a package's native code directly from its respective folder in the `packages/` directory or by opening `bare-expo` in a native editor:
59+
- Android Studio: `yarn edit:android`
60+
- Xcode: `yarn edit:ios`
61+
- Remember to **rebuild** the native project whenever you make a native change
3462

35-
### On maintainable code
63+
### Extra Credit
3664

37-
Code is much more expensive to maintain than it is to write. A maintainable PR is much more likely to be accepted.
65+
- The React Native dev tools are currently disabled in our fork [#5602](https://github.com/expo/expo/issues/5602). You can hack around this by cloning React Native outside this repo, then copying the contents `react-native/React/DevSupport` into `expo/react-native-lab/react-native/React/DevSupport` (this will only enable the shake gesture, CMD+R won't work yet).
66+
- We use a fork of `react-native` in this repo; this fork is located at `react-native-lab/react-native` (you can make changes or cherry-picks from here if you want). It diverges the minimal amount necessary from the `react-native` version in its `package.json`.
67+
- All of the package's `build/` code should be committed. This is because it is simpler to reproduce issues if all contributors are running the same code and so we don't need to rebuild dozen of packages locally on every `git pull` or `git checkout` operation.
68+
- We use a unified set of basic Bash scripts and configs called `expo-module-scripts` to ensure everything runs smoothly (TypeScript, Babel, Jest, etc...).
3869

39-
A maintainable PR is simple to understand and often small in scope. It is robust and unlikely to break if another part of the system is modified. It keeps related code close together and avoids prematurely separating concerns. It follows the coding standards implied by the codebase and Expo coding guidelines. It strikes a balance with enough code to provide a feature that's widely useful without being overly generalized. A maintainable PR minimizes the attention it needs as the codebase changes over time.
70+
## ⏱ Testing Your Changes
4071

41-
Tests and types can improve maintainability and we expect PRs to include them. In particular, use tests to demonstrate the behavior of edge cases that are less likely to occur than the common code path. It is the edge cases we are less likely to notice if they break, and it is the edge cases that we need to behave correctly when they expose an issue in an app and the developer needs to debug. It is relatively easy to get code working; write tests to keep the code working.
72+
> You'll need write about how you tested your changes in the PR under the **Test Plan** section.
4273
43-
However, tests and types can also obstruct maintainability. Overfitted tests break more often and are more difficult to update even when refactoring code that doesn't change its public API. They consume time and attention. Some APIs don't lend themselves well to static typing and lead to precarious type definitions that are not simple to understand or modify. We use tests and types as a means to an end, not an end to zealously pursue.
74+
The best way to get your changes merged is to build good tests for them! We have three different kinds of tests: unit-tests, automated E2E tests, and demos (adding tests that you notice are missing is a great way to become my friend 🥳)!
75+
76+
### ✅ Unit Testing
77+
78+
1. Create a test for your feature in the appropriate package's `src/__tests__` directory (if the file doesn't exist already, create it with the `*-test.ts` or `*-test.tsx` extension).
79+
2. Run the test with `yarn test` and ensure it handles all platforms (iOS, Android, and web). If the feature doesn't support a platform, then you can exclude it by putting your test in a file with a platform extension like: `-test.ios.ts`, `-test.native.ts`, `-test.web.ts`...
80+
3. You can also test platforms one at a time by pressing `X` and selecting the platform you want to test!
81+
82+
### 🏁 E2E Testing
83+
84+
1. Write your tests in `apps/test-suite/tests`
85+
- These tests are written with a non-feature-complete version of Jasmine that runs on the Android and iOS clients, so no special features like snapshot testing will be available.
86+
- If you created a new test file, be sure to add it in `apps/test-suite/TestUtils.js`. This is where you can do platform exclusion. Use `global.DETOX` to test for iOS tests, and `ExponentTest.isInCI` to test for Android Device Farm.
87+
2. Run your tests locally from the `bare-expo` directory with `yarn test:ios`, or `yarn test:web`.
88+
<!-- TODO(Bacon): Remove once Android Detox is setup -->
89+
- For the moment Android Detox is not set up, but you can still run the project in an emulator or on a device to test it.
90+
- It's important you test locally because native CI tests can be fragile, take a while to finish, and be frustrating when they fail.
91+
- When testing for web, you can set `headless: false` in the `apps/bare-expo/jest-puppeteer.config.js` to watch the tests live. You can also execute `await jestPuppeteer.debug();` in `apps/bare-expo/e2e/TestSuite-test.web.js` to pause the tests and debug them!
92+
3. Remember to try and get your feature running on as many platforms as possible.
93+
94+
Thanks again for helping to make sure that Expo is stable for everyone!
95+
96+
## 📚 Updating Documentation
97+
98+
Our docs are made with [Next.js](https://github.com/zeit/next.js). They're located in the `docs/` directory. For more information look at the [`docs/readme.md`](/docs/README.md).
99+
100+
**TL;DR:**
101+
102+
1. Navigate to the `docs/` directory and run `yarn`.
103+
2. Start the project with `yarn dev` (make sure you don't have another server running on port `3000`).
104+
3. Navigate to the docs you want to edit: `cd docs/pages/versions/unversioned/`
105+
4. If you update an older version, ensure the relevant changes are copied into `unversioned/`
106+
107+
## 📝 Writing a Commit Message
108+
109+
> If this is your first time committing to a large public repo, you could look through this neat tutorial: ["How to Write a Git Commit Message"](https://chris.beams.io/posts/git-commit/)
110+
111+
Commit messages are most useful when formatted like so: `[platform][api] Title`. For example if you fix a bug in the package `expo-video` for iOS, you could write: `[ios][video] Fixed black screen bug that appears on older devices`.
112+
113+
## 🔎 Before Submitting
114+
115+
To help keep CI green, please make sure of the following:
116+
117+
- Remember to add a concise description of the change to [CHANGELOG.md](/CHANGELOG.md). This is especially helpful for breaking changes!
118+
- If you modified anything in `packages/`:
119+
- You transpiled the TypeScript with `yarn build` in the directory of whichever package you modified.
120+
- Run `yarn lint --fix` to fix the formatting of the code. Ensure that `yarn lint` succeeds without errors or warnings.
121+
- Run `yarn test` to ensure all existing tests pass for that package, along with any new tests you would've written.
122+
- All `console.log`s or commented out code blocks are removed! :]
123+
- If you edited the `docs/`:
124+
- Any change to the current SDK version should also be in the unversioned copy as well. Example:
125+
- You fixed a typo in `docs/pages/versions/vXX.0.0/sdk/app-auth.md`
126+
- Ensure you copy that change to: `docs/pages/versions/unversioned/sdk/app-auth.md`
127+
- You don't need to run the docs tests locally. Just ensure the links you include aren't broken, and the format is correct!
128+
129+
### Extra Credit
130+
131+
- Our CI tests will finish early if you didn't make changes to certain directories. If you want to **get results faster** then you should make changes to `docs/` in one PR, and changes to anything else in another!

0 commit comments

Comments
 (0)