WIP: Add website contribution workflow#108
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| } | ||
| const thresholdAlong = end === "primary" ? primaryThresholdAlong : secondaryThresholdAlong; | ||
| const direction = end === "primary" ? 1 : -1; | ||
| const threshold = end === "primary" ? primaryThreshold : secondaryThreshold; |
| return runwayZone(runway, lightType, "Point", { point }, extra); | ||
| } | ||
|
|
||
| function runwayPolygonZone(runway, lightType, vertices, extra = {}) { |
Greptile SummaryThis PR introduces a full client-side draft BARS XML generator: a Web Worker reads locally selected scenery packages (.bgl and XML files) via virtual filesystem shims that map
Confidence Score: 4/5Safe to continue iterating on; no data-loss or correctness defects found in the core generation pipeline. The new generation pipeline (worker, virtual FS, BGL parser, matching engine, XML output) is architecturally clean and the Worker lifecycle is handled correctly in XMLGenerator.jsx. The three findings are all non-blocking quality issues: a misleading breadcrumb when skipping the Draft step, a redundant Blob size calculation, and blanket ESLint exclusion of the most complex files. Given this is explicitly a WIP/draft PR, these are reasonable to address before merging. eslint.config.js (blanket extractor ignore removes lint coverage from bgl.js and extract.js), src/pages/ContributeTest.jsx (unconditional Draft breadcrumb and redundant Blob initialiser). Important Files Changed
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
eslint.config.js:10-13
**Blanket ignore removes lint coverage from complex parsing code**
The extractor and shims directories are excluded entirely to avoid false positives from `node:` imports, but this also removes ESLint coverage from the most complex files in the PR (`bgl.js` at ~2100 lines, `extract.js` at ~2150 lines). A narrower fix would configure a second config entry for those directories with `globals.node` added — that suppresses the browser-globals false positives while still catching undefined variables, dead code, and logic errors in the binary parsing routines.
### Issue 2 of 3
src/pages/ContributeTest.jsx:305-309
**"Draft" breadcrumb shown regardless of navigation path**
The `Draft` breadcrumb item is now always rendered on the Test page. When a user reaches the Test page via the Map's "Continue to Next Step" button (which navigates directly to `/contribute/test/${icao}`), the breadcrumb incorrectly shows a Draft Generator step the user never visited. The breadcrumb trail becomes Airport → Map → **Draft** → Test even though the user's actual path was Airport → Map → Test. Consider conditionally rendering this item only when `location.state?.draftXml` is present (i.e., the user actually came from the generator).
### Issue 3 of 3
src/pages/ContributeTest.jsx:44-46
**`originalFileSize` initializer is dead code**
`originalFileSize` is initialised with `new Blob([incomingDraftXml]).size`, but that value is immediately overwritten inside `handleTestXml` with `selectedFile.size` on a successful validation response (and the state is never read before the first test completes). The Blob allocation is wasted work. Since `selectedFile` is already initialised to `new File([incomingDraftXml], ...)`, the initial value could simply be `incomingDraftXml ? new File([incomingDraftXml], incomingDraftFileName, { type: 'application/xml' }).size : 0` — or just `0` — with no observable difference.
Reviews (1): Last reviewed commit: "feat: add website contribution workflow" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| 'dist', | ||
| 'src/features/draft-generator/extractor/**', | ||
| 'src/features/draft-generator/shims/**', | ||
| ], |
There was a problem hiding this comment.
The extractor and shims directories are excluded entirely to avoid false positives from node: imports, but this also removes ESLint coverage from the most complex files in the PR (bgl.js at ~2100 lines, extract.js at ~2150 lines). A narrower fix would configure a second config entry for those directories with globals.node added — that suppresses the browser-globals false positives while still catching undefined variables, dead code, and logic errors in the binary parsing routines.
Prompt To Fix With AI
This is a comment left during a code review.
Path: eslint.config.js
Line: 10-13
Comment:
**Blanket ignore removes lint coverage from complex parsing code**
The extractor and shims directories are excluded entirely to avoid false positives from `node:` imports, but this also removes ESLint coverage from the most complex files in the PR (`bgl.js` at ~2100 lines, `extract.js` at ~2150 lines). A narrower fix would configure a second config entry for those directories with `globals.node` added — that suppresses the browser-globals false positives while still catching undefined variables, dead code, and logic errors in the binary parsing routines.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| <Breadcrumb> | ||
| <BreadcrumbItem title="Airport" link="/contribute/new" /> | ||
| <BreadcrumbItem title="Map" link={`/contribute/map/${icao}`} /> | ||
| <BreadcrumbItem title="Draft" link={`/contribute/generator/${icao}`} /> | ||
| <BreadcrumbItem title="Test" /> |
There was a problem hiding this comment.
The Draft breadcrumb item is now always rendered on the Test page. When a user reaches the Test page via the Map's "Continue to Next Step" button (which navigates directly to /contribute/test/${icao}), the breadcrumb incorrectly shows a Draft Generator step the user never visited. The breadcrumb trail becomes Airport → Map → Draft → Test even though the user's actual path was Airport → Map → Test. Consider conditionally rendering this item only when location.state?.draftXml is present (i.e., the user actually came from the generator).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/pages/ContributeTest.jsx
Line: 305-309
Comment:
**"Draft" breadcrumb shown regardless of navigation path**
The `Draft` breadcrumb item is now always rendered on the Test page. When a user reaches the Test page via the Map's "Continue to Next Step" button (which navigates directly to `/contribute/test/${icao}`), the breadcrumb incorrectly shows a Draft Generator step the user never visited. The breadcrumb trail becomes Airport → Map → **Draft** → Test even though the user's actual path was Airport → Map → Test. Consider conditionally rendering this item only when `location.state?.draftXml` is present (i.e., the user actually came from the generator).
How can I resolve this? If you propose a fix, please make it concise.| const [originalFileSize, setOriginalFileSize] = useState(() => | ||
| incomingDraftXml ? new Blob([incomingDraftXml]).size : 0 | ||
| ); |
There was a problem hiding this comment.
originalFileSize initializer is dead code
originalFileSize is initialised with new Blob([incomingDraftXml]).size, but that value is immediately overwritten inside handleTestXml with selectedFile.size on a successful validation response (and the state is never read before the first test completes). The Blob allocation is wasted work. Since selectedFile is already initialised to new File([incomingDraftXml], ...), the initial value could simply be incomingDraftXml ? new File([incomingDraftXml], incomingDraftFileName, { type: 'application/xml' }).size : 0 — or just 0 — with no observable difference.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/pages/ContributeTest.jsx
Line: 44-46
Comment:
**`originalFileSize` initializer is dead code**
`originalFileSize` is initialised with `new Blob([incomingDraftXml]).size`, but that value is immediately overwritten inside `handleTestXml` with `selectedFile.size` on a successful validation response (and the state is never read before the first test completes). The Blob allocation is wasted work. Since `selectedFile` is already initialised to `new File([incomingDraftXml], ...)`, the initial value could simply be `incomingDraftXml ? new File([incomingDraftXml], incomingDraftFileName, { type: 'application/xml' }).size : 0` — or just `0` — with no observable difference.
How can I resolve this? If you propose a fix, please make it concise.2 participants
Summary
Adds an end-to-end website contribution workflow that builds draft BARS XML from locally selected scenery packages. Extraction and matching run client-side in a Web Worker, so scenery files are not uploaded to an application endpoint.
Changes Made
Additional Information
git diff --checkpassed.pnpm test,pnpm lint, andpnpm buildcould not complete because the existingnode_modulesmixed package-manager state leftvite.exelocked; pnpm dependency repair then failed during automatic registry access.Author Information
Discord Username:
VATSIM CID: 1658308
Checklist: