|
1 | | -# How to review the four upstream PRs before they are sent |
| 1 | +# The upstream submissions - what is out there, and how to follow up |
2 | 2 |
|
3 | | -Everything for the review is in **`fkclaude/docs/upstream-prs/`** - one `.diff` (the code) and |
4 | | -one `.msg.md` (the commit message, which is also the PR description) per PR. Nothing has been |
5 | | -pushed yet; the branches exist only locally. |
| 3 | +**Submitted 2026-07-28.** The four bootloader PRs are open against `oltaco`, and two further |
| 4 | +changes are prepared but deliberately not sent. Current state: |
| 5 | + |
| 6 | +| what | where | status | |
| 7 | +|---|---|---| |
| 8 | +| recover serial/USB DFU after a failed flash | [oltaco #47](https://github.com/oltaco/Adafruit_nRF52_Bootloader_OTAFIX/pull/47) | open, `mergeable=clean` | |
| 9 | +| reboot when a started transfer goes silent | [oltaco #48](https://github.com/oltaco/Adafruit_nRF52_Bootloader_OTAFIX/pull/48) | open, `mergeable=clean` | |
| 10 | +| accept a new session after an interrupted transfer | [oltaco #49](https://github.com/oltaco/Adafruit_nRF52_Bootloader_OTAFIX/pull/49) | open, `mergeable=clean` | |
| 11 | +| serial reboot command (`DFU_REBOOT_PACKET`) | [oltaco #50](https://github.com/oltaco/Adafruit_nRF52_Bootloader_OTAFIX/pull/50) | open, `mergeable=clean` | |
| 12 | +| CI onto Node 24 actions | branch `feature/ci-node24-actions` | **prepared, not sent** | |
| 13 | +| touch 2400 for UF2 mode | `adafruit/Adafruit_TinyUSB_Arduino`, branch in the scratchpad | **prepared, not sent** | |
| 14 | +| serial CLI lockup (a separate project) | [meshcore-dev #2978](https://github.com/meshcore-dev/MeshCore/pull/2978) | open since 2026-07-18, nudged | |
| 15 | + |
| 16 | +Each PR cross-references the other three in order of importance, and #47/#48 are linked to |
| 17 | +upstream issues #40 and #41 with `Related to` - deliberately not `Fixes`, since neither issue is |
| 18 | +fully answered. Comments explaining what is and is not covered are posted under both issues. |
| 19 | + |
| 20 | +Everything for reading them is in **`fkclaude/docs/upstream-prs/`** - one `.diff` (the code) and |
| 21 | +one `.msg.md` (the commit message, which is also the PR description) per change. |
6 | 22 |
|
7 | 23 | Related: `fkclaude/fcl_readme_dfu_recovery.md` (what the changes do), `fkclaude/scripts/README.md` |
8 | 24 | (the test rig), `fkclaude/docs/2026-07-28-dfu-recovery-investigation.md` (how it was diagnosed, |
@@ -143,30 +159,50 @@ transiently (the PR 2 stall timeout is the backstop), and the nRF52832 path is c |
143 | 159 | this fork - no board here selects a non-nRF52840 MCU - so PR 2's effect on it is argued |
144 | 160 | arithmetically, not measured. |
145 | 161 |
|
146 | | -## Step 5 - when you approve, this is what gets run |
| 162 | +## Step 5 - how these were submitted, and how to follow up |
147 | 163 |
|
148 | | -Nothing below has been run yet. |
| 164 | +`gh` is **not installed system-wide**; a portable copy lives at `D:\FkDev\GHcli\bin\gh.exe` |
| 165 | +(v2.96.0, plain zip, no installer). It authenticates from the Git Credential Manager token: |
149 | 166 |
|
150 | 167 | ```sh |
151 | | -for b in dfu-recover-on-usb dfu-stall-timeout dfu-retry-after-abort dfu-serial-reboot-command; do |
152 | | - git push -u origin feature/$b |
153 | | -done |
154 | | - |
155 | | -gh pr create --repo oltaco/Adafruit_nRF52_Bootloader_OTAFIX --base dev \ |
| 168 | +GH=/d/FkDev/GHcli/bin/gh.exe |
| 169 | +export GH_TOKEN=$(printf "protocol=https\nhost=github.com\n\n" | git credential fill | sed -n 's/^password=//p') |
| 170 | +"$GH" pr create -R oltaco/Adafruit_nRF52_Bootloader_OTAFIX --base dev \ |
156 | 171 | --head fkallay1:feature/dfu-recover-on-usb \ |
157 | | - --title "dfu: recover serial/USB DFU automatically after a failed flash" \ |
158 | | - --body-file fkclaude/docs/upstream-prs/dfu-recover-on-usb.msg.md |
| 172 | + --title "$(git log -1 --format=%s feature/dfu-recover-on-usb)" \ |
| 173 | + --body-file fkclaude/docs/upstream-prs/dfu-recover-on-usb.body |
159 | 174 | ``` |
160 | 175 |
|
161 | | -...and the same for the other three. Two notes on the `gh` calls: |
| 176 | +Two things that cost time and are worth keeping: |
162 | 177 |
|
163 | | -- `--body-file` would include the subject line as the first line of the body. Strip it, or pass |
164 | | - `--title` and a body file without the first two lines. |
165 | | -- Open PR 1 first and reference it from the others, so the maintainer sees the intended reading |
166 | | - order. |
| 178 | +- The body must not repeat the title. Split the commit message: `--format=%s` for the title, |
| 179 | + `--format=%b` for the body. |
| 180 | +- **`gh pr edit` fails on this token** - it goes through GraphQL, which demands the `read:org` |
| 181 | + scope even to change a body. The REST path works with the plain `repo` scope: |
| 182 | + `gh api repos/OWNER/REPO/pulls/N --method PATCH --input payload.json`. |
167 | 183 |
|
168 | | -## If you want a change |
| 184 | +To push a change to an open PR, commit on the branch and push normally - the PR follows the |
| 185 | +branch. The branches are regenerated from pristine `upstream/dev` by a script rather than |
| 186 | +edited in place, so a wording or code change means rebuilding that one branch; the other three |
| 187 | +keep their SHAs. |
| 188 | + |
| 189 | +## A mistake worth not repeating |
| 190 | + |
| 191 | +**Never write `#NN` in a commit message on a branch pushed to this fork.** The fork is in |
| 192 | +`oltaco`'s fork network, so GitHub turns every `#NN` in a pushed commit into a `referenced` |
| 193 | +event on that upstream PR, with the commit subject on display. Two internal `fkclaude:` commits |
| 194 | +mentioning `#47` and `#50` therefore leaked working notes into the timelines of PR #47 and #50. |
| 195 | + |
| 196 | +It cannot be undone. Rewriting the messages and force-pushing stops *new* references, but the |
| 197 | +events already recorded stay - GitHub keeps them after the commit becomes unreachable, the |
| 198 | +orphaned commit is still fetchable by SHA so the link still opens it, and the REST API has no |
| 199 | +endpoint to delete a timeline event. Only GitHub Support can purge that. |
| 200 | + |
| 201 | +Write `PR 50`, not `#50`. Check before pushing: |
| 202 | + |
| 203 | +```sh |
| 204 | +git log --format=%B origin/master..HEAD | grep -oE '#[0-9]+' |
| 205 | +``` |
169 | 206 |
|
170 | | -Say which PR and what should differ. The branches are regenerated from pristine `upstream/dev` |
171 | | -by a script rather than edited in place, so a wording or code change means rebuilding that one |
172 | | -branch - the other three keep their SHAs. |
| 207 | +File *content* is safe - a `#NN` inside a committed document creates nothing. Only commit |
| 208 | +messages, PR/issue bodies and comments autolink. |
0 commit comments