Name branch names as a public surface in the disclosure rules - #159
Name branch names as a public surface in the disclosure rules#159jeff-gusto wants to merge 3 commits into
Conversation
The "This repo is public" section enumerates the surfaces a change publishes, and branch names weren't among them. A branch name is on the PR page the moment you push, and unlike a commit subject or PR title it can't be edited at all - renaming means recreating the PR - so the ticket-key rule needs to name it alongside the other permanent surfaces. Docs only; no code or behavior change. Signed-off-by: Jeff Stephens <jeff.stephens@gusto.com>
| - **Don't name internal tooling.** Whatever Gusto runs for analytics, BI, data warehousing, observability, logging, alerting, CI, secrets, or feature flags - name the capability, not the vendor. Same for internal dashboards, internal service and queue identifiers, and internal repo or package names. Naming any of it tells an outside reader nothing they can use while disclosing how Gusto is wired up. Write "groups cleanly in request-log analytics", not the product; "our request logs", not the service the logs land on. (This bullet deliberately names no vendors - listing the current stack here would be the same disclosure it's warning about.) | ||
| - **Don't name internal hosts or environments.** The production and demo API hosts are already public - `README.md` names both. Internal dev, staging, and preview hostnames are not - say "a local development environment" and leave the host out. | ||
| - **Don't link what an outsider can't open.** No SSO- or VPN-gated links that you write yourself: dashboards, log queries, internal docs, ticket trackers. A bare ticket key is the exception, and only in the PR description and its "Linked issue" field - both stay editable, and the key alone opens nothing. Keep keys out of commit subjects and PR titles, which are permanent, and out of source, comments, tests, and docs, where a reader who can't resolve one just hits a dead reference. One thing you don't control: the ticket integration appends its own reference link to the PR body. That's the tooling rather than a choice - leave it, and don't read it as license to add gated links by hand. | ||
| - **Don't link what an outsider can't open.** No SSO- or VPN-gated links that you write yourself: dashboards, log queries, internal docs, ticket trackers. A bare ticket key is the exception, and only in the PR description and its "Linked issue" field - both stay editable, and the key alone opens nothing. Keep keys out of branch names, commit subjects, and PR titles, which are permanent - a branch name is on the PR page from the moment you push, and renaming it means recreating the PR - and out of source, comments, tests, and docs, where a reader who can't resolve one just hits a dead reference. One thing you don't control: the ticket integration appends its own reference link to the PR body. That's the tooling rather than a choice - leave it, and don't read it as license to add gated links by hand. |
There was a problem hiding this comment.
CONTRIBUTING.md:99 still has the narrow version - "an internal ticket key belongs in the PR description, not the commit subject". #155 added both lines together so they've matched until now, and that one's what a human contributor actually reads. I'd add branch names there in this PR.
There was a problem hiding this comment.
Fixed in 026f739. CONTRIBUTING.md:99 now reads "an internal ticket key belongs in the PR description, not the branch name, the commit subject, the PR title, or the issue you reference" - I added PR titles and issues there too, since the narrow version was missing all four and the point is for the two copies to match.
| - **Don't name internal tooling.** Whatever Gusto runs for analytics, BI, data warehousing, observability, logging, alerting, CI, secrets, or feature flags - name the capability, not the vendor. Same for internal dashboards, internal service and queue identifiers, and internal repo or package names. Naming any of it tells an outside reader nothing they can use while disclosing how Gusto is wired up. Write "groups cleanly in request-log analytics", not the product; "our request logs", not the service the logs land on. (This bullet deliberately names no vendors - listing the current stack here would be the same disclosure it's warning about.) | ||
| - **Don't name internal hosts or environments.** The production and demo API hosts are already public - `README.md` names both. Internal dev, staging, and preview hostnames are not - say "a local development environment" and leave the host out. | ||
| - **Don't link what an outsider can't open.** No SSO- or VPN-gated links that you write yourself: dashboards, log queries, internal docs, ticket trackers. A bare ticket key is the exception, and only in the PR description and its "Linked issue" field - both stay editable, and the key alone opens nothing. Keep keys out of commit subjects and PR titles, which are permanent, and out of source, comments, tests, and docs, where a reader who can't resolve one just hits a dead reference. One thing you don't control: the ticket integration appends its own reference link to the PR body. That's the tooling rather than a choice - leave it, and don't read it as license to add gated links by hand. | ||
| - **Don't link what an outsider can't open.** No SSO- or VPN-gated links that you write yourself: dashboards, log queries, internal docs, ticket trackers. A bare ticket key is the exception, and only in the PR description and its "Linked issue" field - both stay editable, and the key alone opens nothing. Keep keys out of branch names, commit subjects, and PR titles, which are permanent - a branch name is on the PR page from the moment you push, and renaming it means recreating the PR - and out of source, comments, tests, and docs, where a reader who can't resolve one just hits a dead reference. One thing you don't control: the ticket integration appends its own reference link to the PR body. That's the tooling rather than a choice - leave it, and don't read it as license to add gated links by hand. |
There was a problem hiding this comment.
Separate thing on the same line: this sentence is hard to follow now. The branch-name aside splits "out of branch names, commit subjects, and PR titles" from the "and out of source, comments, tests, and docs" it pairs with, and hyphens don't read as brackets. I'd pull the rationale into its own sentence:
Keep keys out of branch names, commit subjects, and PR titles, which are permanent, and out of source, comments, tests, and docs, where a reader who can't resolve one just hits a dead reference. Branch names are the strictest of the three: the name is public the moment you push, and renaming it closes the PR.
Also lets you say "public the moment you push" - it's in the branch list before a PR exists at all - and "closes the PR" is GitHub's own wording for it.
There was a problem hiding this comment.
Took the restructure - the rationale is its own sentence now, and each surface group gets its own clause instead of one splitting another. Kept "public the moment you push, before a PR exists at all"; that is the sharper framing.
One change to your wording: I left out "renaming it closes the PR". GitHub's branch-rename retargets open PRs rather than closing them - the close only happens on the git-side route (delete the branch, push a new name), so as a flat statement it would be wrong for the path someone using the rename button takes. Since the reason branch names sit with "permanent" is that a rename does not undo the disclosure either way, I said that instead, which also echoes the section's opening line:
Branch names are the strictest of the permanent three: the name is public the moment you push, before a PR exists at all, and renaming it later doesn't unpublish what GitHub already served.
Happy to put the close back scoped to the delete-and-repush route if you think that route is worth naming.
| ## This repo is public | ||
|
|
||
| `Gusto/gusto-cli` is open source. Source, comments, docs, commit messages, PR titles and descriptions, and review comments are all world-readable and effectively permanent - a force-push cleans up a branch, not the copies GitHub already served. Write everything here for an outside contributor who has no access to Gusto systems. | ||
| `Gusto/gusto-cli` is open source. Source, comments, docs, branch names, commit messages, PR titles and descriptions, and review comments are all world-readable and effectively permanent - a force-push cleans up a branch, not the copies GitHub already served. Write everything here for an outside contributor who has no access to Gusto systems. |
There was a problem hiding this comment.
While you're closing the gap in this list: issue titles and bodies aren't named either, and we point people straight at them - the PR template has a "Linked issue" field and CONTRIBUTING.md says to reference the related GitHub issue. Fine to leave for a follow-on if you want this to stay one surface.
There was a problem hiding this comment.
Pulled it into this PR rather than a follow-on. Issues are named in the world-readable list now ("PR and issue titles and descriptions"), and the ticket-key rule gets its own clause for them: keys stay out of issue titles and bodies, "which anyone browsing the tracker reads whether or not a PR ever links them".
Deliberately did not extend the bare-key carve-out to issue bodies. An issue body is editable like a PR description, so the carve-out's stated reasoning would cover it - but widening what's allowed is a different change from closing a naming gap, so I prefixed the list with "Nothing else gets that exception" and left the exception at the PR description plus the "Linked issue" field. Worth a separate discussion if you think issue bodies should be in it.
Splits the ticket-key rule into one clause per rationale so the branch-name
aside stops interrupting the source/comments/docs pair, and closes the same
gap for GitHub issues - the PR template and CONTRIBUTING both point people at
an issue, but nothing said an issue's title and body are world-readable.
CONTRIBUTING's commit-style line carried the narrow version of the rule
("not the commit subject"); it now matches AGENTS.md, which is the copy a
human contributor actually reads.
Signed-off-by: Jeff Stephens <jeff.stephens@gusto.com>
|
|
||
| - Single-line summary, 20-80 chars, imperative mood | ||
| - Reference the related GitHub issue when there is one; an internal ticket key belongs in the PR description, not the commit subject | ||
| - Reference the related GitHub issue when there is one; an internal ticket key belongs in the PR description, not the branch name, the commit subject, the PR title, or the issue you reference |
There was a problem hiding this comment.
the issue you reference scopes this to linked issues, which is the one case AGENTS.md:66 says doesn't matter - it bans keys in issue titles and bodies "whether or not a PR ever links them". Since the point is for the two copies to match, I'd make it or any issue. It also reads for a beat like it's telling you not to reference the issue, right after the clause telling you to.
| - **Don't name internal tooling.** Whatever Gusto runs for analytics, BI, data warehousing, observability, logging, alerting, CI, secrets, or feature flags - name the capability, not the vendor. Same for internal dashboards, internal service and queue identifiers, and internal repo or package names. Naming any of it tells an outside reader nothing they can use while disclosing how Gusto is wired up. Write "groups cleanly in request-log analytics", not the product; "our request logs", not the service the logs land on. (This bullet deliberately names no vendors - listing the current stack here would be the same disclosure it's warning about.) | ||
| - **Don't name internal hosts or environments.** The production and demo API hosts are already public - `README.md` names both. Internal dev, staging, and preview hostnames are not - say "a local development environment" and leave the host out. | ||
| - **Don't link what an outsider can't open.** No SSO- or VPN-gated links that you write yourself: dashboards, log queries, internal docs, ticket trackers. A bare ticket key is the exception, and only in the PR description and its "Linked issue" field - both stay editable, and the key alone opens nothing. Keep keys out of commit subjects and PR titles, which are permanent, and out of source, comments, tests, and docs, where a reader who can't resolve one just hits a dead reference. One thing you don't control: the ticket integration appends its own reference link to the PR body. That's the tooling rather than a choice - leave it, and don't read it as license to add gated links by hand. | ||
| - **Don't link what an outsider can't open.** No SSO- or VPN-gated links that you write yourself: dashboards, log queries, internal docs, ticket trackers. A bare ticket key is the exception, and only in the PR description and its "Linked issue" field - both stay editable, and the key alone opens nothing. Nothing else gets that exception: keep keys out of branch names, commit subjects, and PR titles, which are permanent; out of issue titles and bodies, which anyone browsing the tracker reads whether or not a PR ever links them; and out of source, comments, tests, and docs, where a reader who can't resolve one just hits a dead reference. Branch names are the strictest of the permanent three: the name is public the moment you push, before a PR exists at all, and renaming it later doesn't unpublish what GitHub already served. One thing you don't control: the ticket integration appends its own reference link to the PR body. That's the tooling rather than a choice - leave it, and don't read it as license to add gated links by hand. |
There was a problem hiding this comment.
"strictest of the permanent three" doesn't hold up: one git push publishes the branch ref and every commit subject on it in the same moment, so "public the moment you push, before a PR exists at all" is just as true of commit subjects. And a subject can't be walked back either - that's an amend plus a force-push, which this section's opening line already says doesn't unpublish what GitHub served.
I'd cut the ranking and keep the mechanic: both go public on push, before a PR exists, and neither a rename nor a force-push undoes it.
| @@ -96,7 +96,7 @@ bun run build:all # macOS arm64 + macOS x86_64 + Linux x86_64 | |||
| ## Commit style | |||
There was a problem hiding this comment.
This bullet is now a four-surface disclosure rule sitting in a commit-style list, and CONTRIBUTING.md never mentions the AGENTS.md section it's mirroring - someone looking for what's safe to put in a branch name isn't going to scan "Commit style" for it. I'd add a pointer to "This repo is public" rather than grow the bullet further; README.md:7 already links AGENTS.md that way.
2 participants
Summary
CONTRIBUTING.md, which said only "not the commit subject". That's the copy a human contributor actually reads.AGENTS.mdand oneCONTRIBUTING.mdbullet. No code, no behavior change.The bare-key carve-out is unchanged and deliberately not widened: keys are still allowed only in the PR description and its "Linked issue" field, not in issue bodies. The note about the reference link the ticket integration appends on its own, and the DCO sign-off carve-out, are untouched.
Linked issue
None - repo hygiene follow-on to #155, which added the section.
Test plan
bun run test:allpasses locally (23 + install + dco + hooks + 90 smoke, 0 fail);typecheck,lint, andformat:checkclean--agentand--humanoutput verified where touched - n/a, no output surface touchedCLAUDE.mdstill resolves through the symlink to the updatedAGENTS.md, so both entry points pick up the changeDCO
git commit -s) per the DCO