Skip to content

Commit a70e962

Browse files
authored
docs: update CONTRIBUTING.md (#1712)
1 parent abb4e88 commit a70e962

6 files changed

Lines changed: 117 additions & 35 deletions

File tree

.github/prettierrc.json

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
11
{
22
"trailingComma": "es5",
3-
"endOfLine": "auto"
3+
"endOfLine": "auto",
4+
"overrides": [
5+
{
6+
"files": ["*.md", "*.mdx"],
7+
"options": {
8+
"organizeImportsSkipDestructiveCodeActions": true,
9+
"proseWrap": "always"
10+
}
11+
}
12+
]
413
}

.swiftformat

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
--swiftversion 5.8
2+
--ifdef no-indent
3+
--stripunusedargs closure-only

CONTRIBUTING.md

Lines changed: 96 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,8 @@ additional questions or comments.
1919

2020
## Commit Messages
2121

22-
This repository adheres to the
23-
[conventional commit format](https://conventionalcommits.org) via
24-
[commitlint](https://github.com/conventional-changelog/commitlint/#what-is-commitlint).
25-
Commit messages must match the pattern:
22+
This repository adheres to the [conventional commit format][] via
23+
[commitlint-lite][]. Commit messages must match the pattern:
2624

2725
```sh
2826
type(scope?): subject
@@ -33,9 +31,11 @@ delimiters.
3331

3432
Following this is necessary to pass CI.
3533

36-
> Note that if you need to push additional changes to an existing PR, you don't
37-
> need to follow this convention. We squash all commits before merging. Only the
38-
> first commit needs to adhere.
34+
> [!NOTE]
35+
>
36+
> If you're pushing additional changes to an existing PR, you don't need to
37+
> follow this convention. We squash all commits before merging. Only the first
38+
> commit needs to adhere.
3939
4040
## Additional Dependencies
4141

@@ -106,9 +106,10 @@ workspace:
106106
open ios/Example.xcworkspace
107107
```
108108

109-
> **Note:** If you made changes to `app.json` or any other assets, you should
110-
> re-run `pod install` to make sure that the changes are included in the Xcode
111-
> project.
109+
> [!NOTE]
110+
>
111+
> If you made changes to `app.json` or any other assets, you should re-run
112+
> `pod install` to make sure that the changes are included in the Xcode project.
112113
113114
### macOS
114115

@@ -133,9 +134,10 @@ workspace:
133134
open macos/Example.xcworkspace
134135
```
135136

136-
> **Note:** If you made changes to `app.json` or any other assets, you should
137-
> re-run `pod install` to make sure that the changes are included in the Xcode
138-
> project.
137+
> [!NOTE]
138+
>
139+
> If you made changes to `app.json` or any other assets, you should re-run
140+
> `pod install` to make sure that the changes are included in the Xcode project.
139141
140142
### Windows
141143

@@ -161,9 +163,11 @@ start windows/Example.sln
161163
If you choose to use Visual Studio, remember to first set the target platform to
162164
`x64`. It may be set to `ARM64` by default.
163165

164-
> **Note:** If you made changes to `app.json` or any other assets, you should
165-
> re-run `install-windows-test-app` to make sure that the changes are included
166-
> in the Visual Studio project.
166+
> [!NOTE]
167+
>
168+
> If you made changes to `app.json` or any other assets, you should re-run
169+
> `install-windows-test-app` to make sure that the changes are included in the
170+
> Visual Studio project.
167171
168172
## Adding New Files
169173

@@ -198,11 +202,48 @@ yarn
198202

199203
## Adding Support For New React Native Versions
200204

201-
First, create a new issue using the "New `react-native` version" template,
202-
update the title, and fill out all the required fields.
205+
First, create a new issue using the _"New `react-native` version"_ template,
206+
update the title and fill out all the required fields. You can find the relevant
207+
discussion link at [`react-native-releases`][].
203208

204-
When opening a PR, link to the issue that was created, and use the table below
205-
to paste in screenshots as you test the different configurations:
209+
Use the [`test-matrix.sh`][] script to both test and capture screenshots. We'll
210+
need the screenshots for the PR we'll create later. For instance, to test 0.73,
211+
run:
212+
213+
```sh
214+
scripts/test-matrix.sh 0.73
215+
```
216+
217+
At the minimum, we should be testing the lowest supported version (0.64 at the
218+
time of writing) in addition to the new version.
219+
220+
As you run the script, you will hit issues. Depending on the root cause, these
221+
are the things that you need to do:
222+
223+
- If the issue is in RNTA or [`@rnx-kit/react-native-host`][]:
224+
- We own these pieces and should fix them ourselves.
225+
- Fixes should go directly to `trunk` if possible.
226+
- If we're adding version specific patches, make sure to add a `TODO` in the
227+
code as well as updating the [Patches page][] in the wiki. This is to make
228+
it easier to identify and remove unused code as we drop support for older
229+
React Native versions.
230+
- Check if others are reporting the same issue in the releases discussion:
231+
- If this is the case, see if they need a minimal repro. This is something we
232+
can easily provide using our example app.
233+
- Otherwise, identify the root cause and file an issue in the relevant
234+
repository, then link to it in the discussion.
235+
- If it's a simple fix, consider fixing it as you already have the context
236+
and it will save time for everyone.
237+
- In any case, always put a link to the relevant comment/issue/PR in the
238+
description of the issue we created at the start of this process.
239+
240+
If the test script succeeds, we are ready to open a PR:
241+
242+
- Update [`package.json`][] to include this new version
243+
- When opening the PR, make sure to link to the issue we created earlier
244+
- Copy and paste the table below into the description, modify it to fit the
245+
scope of the current PR
246+
- The test script we ran should have generated screenshots for the table
206247

207248
```markdown
208249
| Configuration | Android | iOS | macOS | Windows |
@@ -213,12 +254,38 @@ to paste in screenshots as you test the different configurations:
213254
| Fabric + Hermes | TODO | TODO | TODO | TODO |
214255
```
215256

216-
You can use the test script to both test and capture screenshots. For instance,
217-
to test 0.71, run:
218-
219-
```sh
220-
scripts/test-matrix.sh 0.71
221-
```
222-
223-
At the minimum, we should be testing the lowest supported version (0.64 at the
224-
time of writing) in addition to the new version.
257+
While the PR is open:
258+
259+
- Hold off on merging until the release crew has agreed to promote a release
260+
candidate to stable
261+
- Keep an eye on the release discussion for new issues
262+
- Re-run the test script as new release candidates are published and keep the
263+
screenshots up to date
264+
265+
Once the PR is ready to merge:
266+
267+
- Update the [supported versions table][] in the wiki
268+
- Update the appropriate [`@rnx-kit/align-deps`][] profile
269+
270+
For reference, here's the issue (and PR) for 0.73:
271+
https://github.com/microsoft/react-native-test-app/issues/1637
272+
([and PR](https://github.com/microsoft/react-native-test-app/pull/1690))
273+
274+
<!-- References -->
275+
276+
[Patches page]: https://github.com/microsoft/react-native-test-app/wiki/Patches
277+
[`@rnx-kit/align-deps`]:
278+
https://github.com/microsoft/rnx-kit/tree/main/packages/align-deps#contribution
279+
[`@rnx-kit/react-native-host`]:
280+
https://github.com/microsoft/rnx-kit/tree/main/packages/react-native-host#readme
281+
[`package.json`]:
282+
https://github.com/microsoft/react-native-test-app/blob/trunk/package.json
283+
[`react-native-releases`]:
284+
https://github.com/reactwg/react-native-releases/discussions
285+
[`test-matrix.sh`]:
286+
https://github.com/microsoft/react-native-test-app/blob/trunk/scripts/test-matrix.sh
287+
[commitlint-lite]:
288+
https://github.com/microsoft/rnx-kit/tree/main/incubator/commitlint-lite#readme
289+
[conventional commit format]: https://conventionalcommits.org
290+
[supported versions table]:
291+
https://github.com/microsoft/react-native-test-app/wiki#react-native-versions

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# React Native Test App
22

3-
![Contributions Welcome](https://img.shields.io/badge/contributions-welcome-brightgreen) [![Open in Visual Studio Code](https://img.shields.io/static/v1?logo=visualstudiocode&label=&message=Open%20in%20Visual%20Studio%20Code&color=007acc&labelColor=444444&logoColor=007acc)](https://vscode.dev/github/microsoft/react-native-test-app)
3+
![Contributions Welcome](https://img.shields.io/badge/contributions-welcome-brightgreen)
4+
[![Open in Visual Studio Code](https://img.shields.io/static/v1?logo=visualstudiocode&label=&message=Open%20in%20Visual%20Studio%20Code&color=007acc&labelColor=444444&logoColor=007acc)](https://vscode.dev/github/microsoft/react-native-test-app)
45
[![build](https://github.com/microsoft/react-native-test-app/actions/workflows/build.yml/badge.svg?event=push)](https://github.com/microsoft/react-native-test-app/actions/workflows/build.yml)
56
[![npm version](https://img.shields.io/npm/v/react-native-test-app)](https://www.npmjs.com/package/react-native-test-app)
67

example/ios/ExampleTests/ManifestTests.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ class ManifestTests: XCTestCase {
8484
"""
8585

8686
guard let data = json.data(using: .utf8),
87-
let (manifest, checksum) = Manifest.from(data: data) else {
87+
let (manifest, checksum) = Manifest.from(data: data)
88+
else {
8889
XCTFail("Failed to read manifest")
8990
return
9091
}
@@ -155,7 +156,8 @@ class ManifestTests: XCTestCase {
155156
guard let data = json.data(using: .utf8),
156157
let (manifest, _) = Manifest.from(data: data),
157158
let component = manifest.components?.first,
158-
let initialProperties = component.initialProperties else {
159+
let initialProperties = component.initialProperties
160+
else {
159161
XCTFail("Failed to read manifest")
160162
return
161163
}

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@
6161
},
6262
"scripts": {
6363
"format:c": "clang-format -i $(git ls-files '*.cpp' '*.h' '*.m' '*.mm')",
64-
"format:js": "prettier --write $(git ls-files '*.[cm]js' '*.[jt]s' '*.tsx' '*.yml' 'test/**/*.json' ':!:.yarn/releases')",
65-
"format:swift": "swiftformat --swiftversion 5.8 --ifdef no-indent --stripunusedargs closure-only ios macos",
64+
"format:js": "prettier --write --log-level error $(git ls-files '*.[cm]js' '*.[jt]s' '*.tsx' '*.yml' 'CONTRIBUTING.md' 'README.md' 'test/**/*.json' ':!:.yarn/releases')",
65+
"format:swift": "swiftformat $(git ls-files '*.swift')",
6666
"generate:code": "node scripts/generate-manifest.mjs",
6767
"generate:docs": "node scripts/generate-manifest-docs.mjs",
6868
"generate:schema": "node scripts/generate-schema.mjs",

0 commit comments

Comments
 (0)