|
| 1 | +# PR Review |
| 2 | + |
| 3 | +A browser-only PR review tool for [OctopusDeploy/Library](https://github.com/OctopusDeploy/Library). |
| 4 | +Successor to [Hyponome](https://github.com/hnrkndrssn/hyponome): same feature set, no server, |
| 5 | +deployed as a static site to GitHub Pages. |
| 6 | + |
| 7 | +Live at <https://octopusdeploy.github.io/Library/pr-review/>. |
| 8 | + |
| 9 | +## What it does |
| 10 | + |
| 11 | +- Lists open pull requests on `OctopusDeploy/Library`. |
| 12 | +- For each changed file, shows the **before/after side-by-side** in Monaco's |
| 13 | + `DiffEditor`. The before/after contents are fetched directly from |
| 14 | + `raw.githubusercontent.com` (a static CDN that does not count against the |
| 15 | + GitHub API rate limit), not reconstructed from the unified-diff patch. |
| 16 | +- For step-template JSONs, surfaces one **tab per embedded script** that |
| 17 | + changed in the PR, each with its own side-by-side syntax-highlighted diff. |
| 18 | + Supported properties: |
| 19 | + - `Octopus.Action.Script.ScriptBody` (syntax from `Octopus.Action.Script.Syntax`) |
| 20 | + - `Octopus.Action.Terraform.Template` |
| 21 | + - `Octopus.Action.Terraform.VariableValues` |
| 22 | + - `Octopus.Action.CustomScripts.{PreDeploy,Deploy,PostDeploy}.{ps1,sh,csx,fsx,…}` — |
| 23 | + language inferred from the extension. |
| 24 | + |
| 25 | + Scripts that exist in both before and after but didn't actually change in |
| 26 | + this PR don't get a tab (no point looking at an unchanged diff). |
| 27 | + |
| 28 | +## How it differs from Hyponome |
| 29 | + |
| 30 | +| | Hyponome | PR Review | |
| 31 | +|---|---|---| |
| 32 | +| Runtime | ASP.NET Core (server) | Static SPA (browser only) | |
| 33 | +| GitHub access | Octokit, server-side PAT | `fetch` to `api.github.com`, unauthenticated | |
| 34 | +| Hosting | Docker container | GitHub Pages | |
| 35 | +| Diff viewer | Ace + ace-diff | Monaco `DiffEditor` | |
| 36 | +| UI | Bootstrap 3 + jQuery | React 18 + plain CSS | |
| 37 | + |
| 38 | +## Authentication |
| 39 | + |
| 40 | +None. The site uses the GitHub REST API unauthenticated, which is rate-limited to |
| 41 | +**60 requests per hour per IP address**. Each PR detail view consumes roughly 3 |
| 42 | +requests (PR + files + occasionally the raw diff for files with omitted patches), |
| 43 | +so casual browsing of ~15 PRs per hour fits comfortably. |
| 44 | + |
| 45 | +When the limit is hit, the site renders an explanatory error state with the |
| 46 | +reset time. There is no PAT prompt and no token storage — keeping the deployment |
| 47 | +truly static and avoiding any browser-stored secrets. |
| 48 | + |
| 49 | +## Local development |
| 50 | + |
| 51 | +```sh |
| 52 | +cd pr-review |
| 53 | +npm install |
| 54 | +npm run dev |
| 55 | +``` |
| 56 | + |
| 57 | +Opens at `http://localhost:5173/Library/pr-review/`. Hot reload works as |
| 58 | +expected via Vite. |
| 59 | + |
| 60 | +Useful scripts: |
| 61 | + |
| 62 | +| Script | Purpose | |
| 63 | +|---|---| |
| 64 | +| `npm run dev` | Vite dev server with HMR | |
| 65 | +| `npm run build` | Type-check, then build a production bundle into `dist/` | |
| 66 | +| `npm run typecheck` | TS only, no emit | |
| 67 | +| `npm run preview` | Serve the production build locally | |
| 68 | + |
| 69 | +## Deployment |
| 70 | + |
| 71 | +Pushes to `master`/`main` that touch `pr-review/**` or the workflow file run |
| 72 | +`.github/workflows/pr-review-deploy.yml`, which builds the site and publishes |
| 73 | +it to GitHub Pages. |
| 74 | + |
| 75 | +**One-time setup** (only needed the first time): |
| 76 | + |
| 77 | +1. In repo *Settings → Pages*, set **Source** to **GitHub Actions**. |
| 78 | +2. Run the workflow once (push or *Run workflow* in the Actions tab). |
| 79 | + |
| 80 | +The site lands at `https://octopusdeploy.github.io/Library/pr-review/`. If |
| 81 | +you change the deployment path, also update `base` in `vite.config.ts` and |
| 82 | +the `_site/pr-review` path in the workflow. |
| 83 | + |
| 84 | +## Code layout |
| 85 | + |
| 86 | +``` |
| 87 | +src/ |
| 88 | +├── main.tsx Entry — mounts <App/>. |
| 89 | +├── App.tsx HashRouter + header + footer. |
| 90 | +├── styles.css All app styling (CSS custom properties, dark mode). |
| 91 | +├── routes/ |
| 92 | +│ ├── PullRequestList.tsx Open PRs list. |
| 93 | +│ └── PullRequestDetail.tsx PR header + file panels. |
| 94 | +├── api/ |
| 95 | +│ ├── github.ts fetch-based client. Lists PRs/files via api.github.com, |
| 96 | +│ │ reads file contents from raw.githubusercontent.com. |
| 97 | +│ └── types.ts Hand-trimmed shapes for the endpoints we use. |
| 98 | +├── lib/ |
| 99 | +│ ├── extractScript.ts Pull every changed Octopus script out of a step-template |
| 100 | +│ │ JSON (ScriptBody, Terraform, custom scripts). |
| 101 | +│ └── languageDetect.ts Filename → Monaco language id; binary detection. |
| 102 | +└── components/ |
| 103 | + ├── PullRequestHeader.tsx Title / branches / author. |
| 104 | + ├── FilePanel.tsx Tabbed per-file panel (File diff + one tab per script). |
| 105 | + ├── ErrorState.tsx Generic + rate-limit-aware error rendering. |
| 106 | + └── TimeAgo.tsx Relative timestamps. |
| 107 | +``` |
| 108 | + |
| 109 | +The repo target (`OctopusDeploy/Library`) is hardcoded in `src/api/github.ts`. |
| 110 | +A fork that wants to point this at a different repo edits one constant. |
| 111 | + |
| 112 | +## Why hash routing? |
| 113 | + |
| 114 | +GitHub Pages is static hosting — any deep link like `/pulls/123` would 404 on |
| 115 | +refresh. `HashRouter` keeps the route entirely in the URL fragment |
| 116 | +(`#/pulls/123`), which the server never sees. Two-route SPA, internal tool, |
| 117 | +zero extra deployment complexity. |
| 118 | + |
| 119 | +## Known harmless console warning |
| 120 | + |
| 121 | +Navigating from one PR to another sometimes logs: |
| 122 | + |
| 123 | +> `Uncaught Error: TextModel got disposed before DiffEditorWidget model got reset` |
| 124 | +
|
| 125 | +This is a known race inside `@monaco-editor/react` where Monaco's lazy CDN |
| 126 | +load completes after React has already unmounted the previous `DiffEditor`. |
| 127 | +It's thrown asynchronously during teardown, doesn't affect any rendered UI, |
| 128 | +and can be ignored. Each `DiffEditor` already gets a stable `key` per |
| 129 | +`file.sha`/`file.filename` to force a clean remount, which suppresses the |
| 130 | +worst of it; the remaining warning is upstream. |
| 131 | + |
0 commit comments