Conversation
GigaHierz
left a comment
There was a problem hiding this comment.
head 2d7bef9 · gate matched headRefOid (no STALE); PR is 1 ahead / 7 behind origin/main, merge-tree clean; mint broken-links --check-redirects pass, check-orphans.sh pass, 5 URLs 200 with 0 redirects, mint dev rendered /home/celo (desktop) and /build-on-celo/index (412x915) with 0 console errors
Verdict: REQUEST-CHANGES
Reviewed on the head SHA, against current main (merge base b7cb920; main has 7 newer commits, none touch the changed hunks).
Findings (severity-ordered)
-
LOW-MEDIUM ·
docs.json:4538(alsocontribute-to-celo/index.mdx:23,home/celo.mdx:141-143) · The PR body says every link now uses "one name", but the navbar/footer link is still labelled "Developer Newsletter", and the pages describe the audience in different words.- Failure scenario: a reader sees "Developer Newsletter" in the footer and "Celo Developers newsletter" in body text and cannot tell they are one list, which is the confusion #1993 set out to remove. The audience line is "Events, hackathons, and funding opportunities for builders on Celo" on
home/celo.mdx, "developer programs, grants, and builder updates" oncontribute-to-celo/index.mdxandbuilders.mdx, and "latest funding opportunities" onfund-your-project.mdx. Whichever is true, the others are wrong. - Fix shape: set the
docs.jsonlabel to "Celo Developers newsletter". Pick one audience sentence and use it in the card and both contribute pages;fund-your-projectcan keep its funding-specific lead-in. - Note: I could not verify that the newsletter carries "hackathons"; the maintainer decision approved the newsletter, not that sentence. Confirm against a recent issue before it ships.
- Failure scenario: a reader sees "Developer Newsletter" in the footer and "Celo Developers newsletter" in body text and cannot tell they are one list, which is the confusion #1993 set out to remove. The audience line is "Events, hackathons, and funding opportunities for builders on Celo" on
-
LOW · PR body,
Closes #1993· #1993 asks to (a) clean up lists that are not updated and (b) "clarify which audience is targeted by the active ones". (a) is done for DevDesk and Tech Sync. (b) is done only for the developer newsletter. The two governance pages (home/protocol/governance/voting-in-governance.mdx:178,home/protocol/governance/voting-in-governance-using-mondo.mdx:34) say "Celo Signal mailing list" with no audience, and the PR leaves them untouched.home/protocol/staking/voting.mdx:19does state it. The issue has no checkbox list.- Failure scenario: the issue auto-closes and the Signal-audience half of the ask is lost.
- Fix shape: either add a half-sentence audience to the two governance bullets (signup and calendar links stay as they are, so the maintainer decision is honoured), or change
Closes #1993toRefs #1993and say the Signal audience line remains.
Verified good
- Decisions honoured. A repo-wide grep (mdx, json, md, js, snippets,
docs.jsonbanner/footer/navbar) on the PR head finds no remainingdevdesk,dev desk,tech sync,techsync, orembeds.beehiiv.com. Every beehiiv link ishttps://celo-devs.beehiiv.com/subscribe:docs.json,home/celo.mdx,build-on-celo/index.mdx,build-on-celo/fund-your-project.mdx,contribute-to-celo/index.mdx,contribute-to-celo/builders.mdx. - All three voting pages keep the Celo Signal signup (
share.hsforms.com/1Qrhush1vSA2WIamd_yL4ow53n4j) and the Signal public calendar link; the diff does not touch them. https://newsletter.celo.org/subscribeis not used anywhere. That is correct: no page needs a general-audience newsletter that Signal does not cover, andhome/celo.mdx's section is "Join the Celo Builder Ecosystem", so the developer newsletter fits.- Tech Sync is fully removed (one line in
contribute-to-celo/release-process/base-cli-contractkit-dappkit-utils.mdx; rendered page has 0 matches). No page was deleted or moved, so no redirect, nav or inbound-link work is needed. - URLs via
curl -sILwith a browser UA:celo-devs.beehiiv.com/subscribe200 (title "Subscribe | Celo Developers"),newsletter.celo.org/subscribe200, Signal form 200,x.com/CeloDevs200,x.com/cLabs200, all 0 redirects. The removed embed URL also returned 200. mint broken-links --check-redirectspasses;bash scripts/check-orphans.shfinds no orphans;gh pr checks: broken-links, GitGuardian, StepSecurity pass, Mintlify deployment skipped.- Rendering:
/home/celo(desktop) and/build-on-celo/index(412x915) show the new card text and link target, 0 console errors (1 pre-existing warning)./build-on-celo/fund-your-projectreturned a 500 in the browser from an ENOENT on mint's generatedsrc/_props/generatedDocsNav.json(dev-server artifact; curl got 200 with the correct subscribe link, and the page change is one link). The shared browser also jumped to a different local port mid-run, so I did not retry it there. Server on port 3457 stopped. - Wording is not promotional: the card text and label state the audience with no superlatives and no comparison to another chain.
Not verified
- The PR body's claim that the embed and celo-devs publication are the same via
embeds.beehiiv.com/api/embeds/...: that URL returns a Cloudflare challenge to curl. The subscribe page title reads "Celo Developers" and the maintainers have since confirmed in the issue thread that the two are the same publication, so this is moot.
What the PR got right
- Minimal one-concern diff (4 files, +5/-5) matching the decision thread: DevDesk removed rather than renamed, Signal left alone, no speculative general-audience link added.
- Reuses the existing
celo-devs.beehiiv.com/subscribeURL, so the navbar/footer and the two contribute pages needed no churn. - Pastes the
mint broken-linksoutput as AGENTS.md section 9 requires. This is one round of small fixes, not a rethink.
Merge-order hazards
- #2364, #2346 and #2344 all edit
docs.json. This PR does not, unless finding 1 is fixed (the label is at about line 4538, in the navbar/footer block).git merge-treeof the head against current main reports no conflicts. If finding 1 touchesdocs.json, rebase after those three land. No open PR touchesbuild-on-celo/fund-your-project.mdx,build-on-celo/index.mdx,home/celo.mdx, the release-process page, or the two contribute pages.
… drop Tech Sync The DevDesk embed and celo-devs.beehiiv.com are the same beehiiv publication; use the subscribe page and one name everywhere. Remove the cLabs Tech Sync mailing list, which has no link or signup. Refs #1993
2d7bef9 to
d7ded45
Compare
|
Both are in d7ded45, rebased onto current main.
|
The docs sent readers to developer news under two labels, "Join the Newsletter" and "DevDesk mailing list", both pointing at a bare beehiiv embed, and the SDK release page listed a "cLabs' Tech Sync" mailing list with no link or signup. The embed is the same beehiiv publication as celo-devs.beehiiv.com, so this points every developer-newsletter link at its subscribe page under one name, the Celo Developers newsletter, with one audience line, and drops the Tech Sync line. The two governance voting pages now say who the Celo Signal list is for, matching the staking voting page.
The audience line, "Events, hackathons, and funding opportunities for builders on Celo", matches what the publication sends: its sitemap lists mostly hackathon, Proof of Ship and bounty issues.
fund-your-projectkeeps its funding-specific lead-in.The Celo Signal signup and calendar links on the three voting pages stay as they are. GigaHierz confirmed that in #1993 (comment), and named https://newsletter.celo.org/subscribe as the general-audience newsletter for pages that need one. None do today.
I confirmed the embed and celo-devs.beehiiv.com are one publication via
https://embeds.beehiiv.com/api/embeds/eeadfef4-2f0c-45ce-801c-b920827d5cd2, andcelo-devs.beehiiv.com/subscribereturns 200.Closes #1993