my-ship-it commented on PR #1971: URL: https://github.com/apache/cloudberry/pull/1971#issuecomment-5628772421
Self-review before asking anyone to spend more time on this. Three problems, all mine, all fixed in 0a5d33a. Two of them would have wasted a reviewer's afternoon. ### 1. The PR referenced a component it did not ship — any build from this branch failed 37 figures are laid out with HTML and CSS rather than drawn as SVG, and are handed to an MDX component as a string, because their markup is not valid JSX: **1,062 inline `style` attributes, 46 void tags, and 2 blocks whose tags are unbalanced.** The component and its stylesheet existed only in my local preview site. The README described them instead of shipping them, so anyone who checked this branch out and pointed Docusaurus at it would have got an undefined component and a failed build — not a missing picture, a broken build. They now ship in `docs/core-internals/_components/`, and each page that needs the component imports it, so **a consuming site registers nothing**. I verified that claim the only way it is worth verifying: by deleting the component registration and emptying `customCss` on my test site and building again. 23 pages, no MDX errors, no broken links, no broken anchors, and all 101 figures present — 64 images and 37 HTML figures. ### 2. One code citation pointed at the wrong directory `src/backend/access/transam.h` should be `src/include/access/transam.h`. The line number was right — `transam.h:33` is the `FrozenTransactionId` block the excerpt quotes. I found it by checking every `title="file:line"` citation against `git ls-tree`: **361 of 370 path fragments resolve exactly.** Of the nine that did not, this was the only one that was wrong. Four are abbreviated inside prose and resolve from context (`tcp/ic_tcp.c`, `appendonly_compaction.c`, two `storage/proto/*.proto`); two name files that only exist in a built tree (`src/include/pg_config.h`, and a pax init script the text itself describes as generated). I can spell the abbreviated four out in full if you would rather they be copy-pasteable. ### 3. 68 highlight markers leaked into the output The handbook wraps a key token in a marker pair, and its renderer turns that into a highlight span. The generator stripped markers **line by line**, which silently misses a pair that opens on one line and closes on the next — and it never touched the markers inside diagram sources or inside the HTML figures at all. Stripping now happens across the whole block: in diagram labels the markers reduce to plain text, and in the HTML figures they become the same highlight span the handbook uses, with the same CSS. Markers remaining under `docs/core-internals`: zero. ### Still open, and I would value an opinion The handbook carries **no incubation notice** — no mention of incubation, of the ASF, or of trademark anywhere in its 652 pages, while it is titled "Apache Cloudberry — Core Internals". Given `DISCLAIMER`, I assume it needs one. I would rather add what the project wants than guess, so: should the incubation disclaimer go on a title page, in the README, in every chapter's front matter, or somewhere else you already have a convention for? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
