docs: exempt maintainers from the public API issue-first rule - #976
Conversation
|
The policy should not merge until maintainer identity and public API detection are expressed using reliable signals. Reviews (1) · Last reviewed commit: "docs: exempt maintainers from the public..." |
| - The main runtime package is `posthog/`; tests live under `posthog/test/`. | ||
| - The project uses `uv` for local development. See `CONTRIBUTING.md` for setup. | ||
| - Before adding or changing public API, follow "Public API changes" in [CONTRIBUTING.md](./CONTRIBUTING.md): the API shape must be agreed on the issue first. For SDK design guidance, read https://posthog.com/handbook/engineering/sdks/guidelines.md. | ||
| - Public API changes (a diff in `references/public_api_snapshot.txt`): if the author is a PostHog maintainer (git email ends in `@posthog.com`), the PR is the discussion, so don't open or suggest an issue. Otherwise, follow "Public API changes" in [CONTRIBUTING.md](./CONTRIBUTING.md): if there's no agreed issue, stop and tell the user. If a PR already exists, add a public-API note to its description and draft an issue body for the user to post. Never open an issue yourself. For SDK design guidance, read https://posthog.com/handbook/engineering/sdks/guidelines.md. |
There was a problem hiding this comment.
Unreliable maintainer identification
The exemption treats an author as a maintainer when their git email ends in @posthog.com, but Client Libraries maintainers commonly use GitHub noreply or personal addresses, while git email is locally configurable. This can send maintainers through the external-contributor workflow and let non-maintainers bypass it. Use authenticated GitHub team membership or explicit PR metadata instead.
Prompt To Fix With AI
This is a comment left during a code review.
Path: AGENTS.md
Line: 10
Comment:
**Unreliable maintainer identification**
The exemption treats an author as a maintainer when their git email ends in `@posthog.com`, but Client Libraries maintainers commonly use GitHub noreply or personal addresses, while git email is locally configurable. This can send maintainers through the external-contributor workflow and let non-maintainers bypass it. Use authenticated GitHub team membership or explicit PR metadata instead.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| - The main runtime package is `posthog/`; tests live under `posthog/test/`. | ||
| - The project uses `uv` for local development. See `CONTRIBUTING.md` for setup. | ||
| - Before adding or changing public API, follow "Public API changes" in [CONTRIBUTING.md](./CONTRIBUTING.md): the API shape must be agreed on the issue first. For SDK design guidance, read https://posthog.com/handbook/engineering/sdks/guidelines.md. | ||
| - Public API changes (a diff in `references/public_api_snapshot.txt`): if the author is a PostHog maintainer (git email ends in `@posthog.com`), the PR is the discussion, so don't open or suggest an issue. Otherwise, follow "Public API changes" in [CONTRIBUTING.md](./CONTRIBUTING.md): if there's no agreed issue, stop and tell the user. If a PR already exists, add a public-API note to its description and draft an issue body for the user to post. Never open an issue yourself. For SDK design guidance, read https://posthog.com/handbook/engineering/sdks/guidelines.md. |
There was a problem hiding this comment.
Snapshot trigger misses changes
Defining a public API change as a diff in the snapshot misses source changes until that file is explicitly regenerated. An agent can add or change an exported option, method, or type, see no snapshot diff, and skip this policy until CI reports the stale snapshot. Define the trigger as changing public API in source and describe the snapshot diff only as a detection signal.
Prompt To Fix With AI
This is a comment left during a code review.
Path: AGENTS.md
Line: 10
Comment:
**Snapshot trigger misses changes**
Defining a public API change as a diff in the snapshot misses source changes until that file is explicitly regenerated. An agent can add or change an exported option, method, or type, see no snapshot diff, and skip this policy until CI reports the stale snapshot. Define the trigger as changing public API in source and describe the snapshot diff only as a detection signal.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
posthog-python Compliance ReportDate: 2026-09-21 14:41:10 UTC ✅ All Tests Passed!111/111 tests passed Capture_V1 Tests✅ 94/94 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
dustinbyrne
left a comment
There was a problem hiding this comment.
The maintainer exemption looks good. One non-blocking clarification about identifying maintainers.
AI-assisted review: source inspection and existing CI; no new tests executed.
| - The main runtime package is `posthog/`; tests live under `posthog/test/`. | ||
| - The project uses `uv` for local development. See `CONTRIBUTING.md` for setup. | ||
| - Before adding or changing public API, follow "Public API changes" in [CONTRIBUTING.md](./CONTRIBUTING.md): the API shape must be agreed on the issue first. For SDK design guidance, read https://posthog.com/handbook/engineering/sdks/guidelines.md. | ||
| - Public API changes (a diff in `references/public_api_snapshot.txt`): if the author is a PostHog maintainer (git email ends in `@posthog.com`), the PR is the discussion, so don't open or suggest an issue. Otherwise, follow "Public API changes" in [CONTRIBUTING.md](./CONTRIBUTING.md): if there's no agreed issue, stop and tell the user. If a PR already exists, add a public-API note to its description and draft an issue body for the user to post. Never open an issue yourself. For SDK design guidance, read https://posthog.com/handbook/engineering/sdks/guidelines.md. |
There was a problem hiding this comment.
Minor (non-blocking): Could we treat @posthog.com as one signal of maintainer status rather than the exclusive test? A confirmed Client Libraries maintainer can use a GitHub noreply address, which sends them through the external-contributor issue-first flow despite the exemption in CONTRIBUTING.md. Consider accepting independently confirmed maintainer identity too.
Problem
Port of PostHog/posthog-js#5044.
The "Public API changes" section in
CONTRIBUTING.mdand the matching line inAGENTS.mdsay the API shape must be agreed on an issue first. Nothing in either file exempts maintainers. So when a Client Libraries maintainer's agent changes public API in their own PR, it doesn't find an agreed issue and opens one. We end up closing duplicate issues and paying for the turns and tokens. For the team, the PR is where we discuss API shape.Changes
CONTRIBUTING.md: says the issue-first rule is for external contributors, and adds a path for contributors who already have a PR open. They call out the public API change at the top of the PR description and link or open an issue to discuss it, instead of stopping.AGENTS.md: agents skip the issue when the author is a PostHog maintainer (git email ends in@posthog.com). For contributors, agents stop and ask, or draft an issue body for the user to post. Agents never open an issue on their own.CONTRIBUTING.md, becauseAGENTS.mdnow covers it.Testing
Docs only. Same wording as posthog-js#5044, which was reviewed and approved.
🤖 Generated with Claude Code