[MAINTENANCE] Surface the RFC threshold in the PR template, AGENTS.md, and a new check - #12043
Merged
Conversation
A contributor opening a pull request for new data source support had no way to learn an RFC was expected: the criteria live in CONTRIBUTING.md, and nothing in the pull-request path pointed at them. The cost lands on the contributor, who finds out only after writing the code. Three changes, aimed at the three places that signal can land: - The pull-request template now states the RFC threshold inline and carries a checklist item for it. It also fixes the AGENTS.md link, which used a relative path and resolved to .github/AGENTS.md rather than the file at the repo root, and normalizes the remaining github.com links onto the current repository path. - AGENTS.md gains a "Before opening a pull request" section. Tools that compose a pull-request body programmatically never load the template, so a template-only fix reaches humans and misses everything else; AGENTS.md is the file those tools do read. - A new pr-hygiene check flags changes that look like new backend support (a new compatibility module, a new fluent datasource, or a new reqs/requirements-dev-* file) and asks for one line in the description: either a link to an accepted RFC or a statement that none is needed. It enforces that the question was answered, never what the answer is, so a false positive costs one line rather than a blocked pull request. It reads pull-request metadata through the API and never checks out pull-request code, so it can run on fork pull requests immediately instead of waiting for a maintainer to approve a CI run. Also removes the legacy Markdown issue templates. They were superseded by the structured issue forms but left in place, so GitHub kept offering all four in the chooser. Because the Markdown templates apply no labels, issues filed through them arrived with neither a type label nor `triage`, silently bypassing the triage queue that the ready-for-work claiming gate depends on.
✅ Deploy Preview for niobium-lead-7998 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
- pr-hygiene no longer checks the title prefix. pr-title-checker.yml already did, with a different prefix list and different matching, so the two could disagree on the same title. Leaving one check canonical also removes the need for the bot-author exemption, which was inert while the other check ran unexempted on the same events. - The RFC declaration regex accepts a list marker, blockquote, emphasis, or code ticks around the token. The guiding comment presents both forms as bullets; a contributor who pasted one verbatim did not satisfy the check that asked for it. - The PR template states the RFC line syntax the check looks for, instead of leaving a contributor to learn it from a bot comment after the fact. - A concurrency group serializes runs per pull request. The comment dedup is a read-then-write, so overlapping runs could both post the guiding comment. - The corporate CLA form link now matches CLA.md everywhere. .clabot and cla-check.yml pointed at a different form than CLA.md and AGENTS.md, so a contributor could sign a form the bot never checks. - The issue chooser's discussion and advisory links point at this repo.
pr-hygiene.yml runs on pull_request_target with no checkout step, so its inline github-script logic can't require() shared code and has no way to be exercised except by opening a pull request against it. This mirrors that logic verbatim into a plain Node test, following the pattern already established for cla-check.yml. Covers the three signal regexes and their near misses, the added-files-only rule, the HTML-comment stripping that keeps an unedited template from counting as a declaration, every dressed form of the declaration line the check accepts, and the overall verdict. The load-bearing case is the round trip: the guiding comment's own remediation lines are extracted from the mirrored comment builder and fed back through the declaration scan, so a reworded comment that the regexes would no longer accept fails the test. A check that rejects the fix it just asked for is the failure mode most worth catching here, and it is invisible to any test that types the accepted forms out by hand. Cases that are accepted on purpose -- a bare 'RFC:' with no link, a declaration in a code block, the bot's comment quoted back -- are asserted as accepted, so tightening the check later is a decision rather than a surprise.
joshua-stauffer
marked this pull request as ready for review
August 13, 2026 09:23
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@abdullah094 opened #12012 adding Exasol support and got told an RFC was required after writing the code. That's on us — the RFC criteria live in
CONTRIBUTING.md, and nothing in the pull-request path pointed at them. I said on that PR I'd fix it for the future; this is that fix.What changed
1. Pull-request template — states the RFC threshold inline (in a comment above the checklist, so the criteria are visible while writing the description) and adds a checklist item for it.
Two drive-by fixes in the same file:
AGENTS.mdlink was relative, so it resolved to.github/AGENTS.md(404) rather than the file at the repo root. Now absolute.github.com/great-expectations/...links normalized tofivetran/..., matchingCONTRIBUTING.mdand the rest of.github/.2.
AGENTS.mdgains a "Before opening a pull request" section. This is the part that matters for tooling.gh pr create --bodydiscards the pull-request template entirely, and nothing injects it into a coding agent's context — so a template-only fix reaches humans opening PRs in the web UI and misses every programmatically-opened PR.AGENTS.mdis the file agents read from the repo root by convention, which makes it the highest-leverage place to put this.3. New
pr-hygienecheck (.github/workflows/pr-hygiene.yml).Flags changes that look like new backend support and asks for one line in the description — either
RFC: <link>orNo RFC needed: <reason>. Signals, all added-file only, so modifying an existing datasource doesn't trip it:great_expectations/compatibility/*.pygreat_expectations/datasource/fluent/*_datasource.pyreqs/requirements-dev-*.txtDesign notes:
pull_request_target, metadata-only. It reads PR metadata through the API and never checks out or executes PR code, so it's safe on fork PRs. That trigger is the point: it fires the moment a PR opens, rather than waiting for a maintainer to approve a CI run — which is exactly the delay that made [FEATURE] Exasol integration #12012 go wrong.4. Removes the legacy Markdown issue templates.
bug_report.mdandfeature_request.mdwere superseded by the structured issue forms but never deleted, so GitHub kept offering all four in the chooser. Because the Markdown templates carrylabels: '', issues filed through them arrived with neither a type label nortriage— silently bypassing the triage queue that theready-for-workclaiming gate depends on.Verification
pre-commit run --files ...— all hooks pass (check-yaml,prettier,end-of-file-fixer,trailing-whitespace)node --checkci/checks/check_repo_root_size.shpasses (no root files added)Not done here
pr-hygieneis not yet a required check. It's a separate workflow, so it can't joinci-required'sneeds:list — that would also put it behind the actor-permission gate and destroy the early-feedback property. Making it blocking is a branch-protection setting; I'd suggest living with it advisory for a few weeks first to see the false-positive rate.pull_request_targetuses the workflow from the base branch, so it goes live on the first PR opened after merge.