diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 08def88577..e708ae41b2 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -6,6 +6,7 @@ - [ ] Bugfix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) +- [ ] New developer-facing feature (adds functionality for component developers; no end-user configuration change) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) — [policy](https://developers.esphome.io/contributing/code/#what-constitutes-a-c-breaking-change) - [ ] Developer breaking change (an API change that could break external components) — [policy](https://developers.esphome.io/contributing/code/#what-is-considered-public-c-api) - [ ] Undocumented C++ API change (removal or change of undocumented public methods that lambda users may depend on) — [policy](https://developers.esphome.io/contributing/code/#c-user-expectations) @@ -20,6 +21,10 @@ - esphome/esphome.io# +**Pull request in [developers.esphome.io](https://github.com/esphome/developers.esphome.io) with developer documentation (if applicable):** + +- esphome/developers.esphome.io# + ## Test Environment - [ ] ESP32 diff --git a/.github/scripts/auto-label-pr/constants.js b/.github/scripts/auto-label-pr/constants.js index 2938fd923c..b95bc17518 100644 --- a/.github/scripts/auto-label-pr/constants.js +++ b/.github/scripts/auto-label-pr/constants.js @@ -22,11 +22,13 @@ module.exports = { 'has-tests', 'needs-tests', 'needs-docs', + 'needs-developer-docs', 'needs-codeowners', 'too-big', 'labeller-recheck', 'bugfix', 'new-feature', + 'new-feature-developer', 'breaking-change', 'developer-breaking-change', 'undocumented-api-change', @@ -40,5 +42,17 @@ module.exports = { // Keep matching the old esphome-docs name during the transition period /https:\/\/github\.com\/esphome\/esphome-docs\/pull\/\d+/, /esphome\/esphome-docs#\d+/ + ], + + DEVELOPER_DOCS_PR_PATTERNS: [ + /https:\/\/github\.com\/esphome\/developers\.esphome\.io\/pull\/\d+/, + /esphome\/developers\.esphome\.io#\d+/ + ], + + // Files whose developer-facing changes are documented via Python docstrings + // only - developers.esphome.io has no reference page for them yet, so PRs + // touching nothing but these files (and tests/) skip needs-developer-docs. + DEV_DOCS_EXEMPT_FILES: [ + 'esphome/config_validation.py' ] }; diff --git a/.github/scripts/auto-label-pr/detectors.js b/.github/scripts/auto-label-pr/detectors.js index 4406370a27..2478ccf959 100644 --- a/.github/scripts/auto-label-pr/detectors.js +++ b/.github/scripts/auto-label-pr/detectors.js @@ -1,4 +1,4 @@ -const { DOCS_PR_PATTERNS } = require('./constants'); +const { DOCS_PR_PATTERNS, DEVELOPER_DOCS_PR_PATTERNS, DEV_DOCS_EXEMPT_FILES } = require('./constants'); const { COMPONENT_REGEX, detectComponents, @@ -245,6 +245,7 @@ async function detectPRTemplateCheckboxes(context) { const checkboxPatterns = [ { pattern: /- \[x\] Bugfix \(non-breaking change which fixes an issue\)/i, label: 'bugfix' }, { pattern: /- \[x\] New feature \(non-breaking change which adds functionality\)/i, label: 'new-feature' }, + { pattern: /- \[x\] New developer-facing feature \(adds functionality for component developers; no end-user configuration change\)/i, label: 'new-feature-developer' }, { pattern: /- \[x\] Breaking change \(fix or feature that would cause existing functionality to not work as expected\)/i, label: 'breaking-change' }, { pattern: /- \[x\] Developer breaking change \(an API change that could break external components\)/i, label: 'developer-breaking-change' }, { pattern: /- \[x\] Undocumented C\+\+ API change \(removal or change of undocumented public methods that lambda users may depend on\)/i, label: 'undocumented-api-change' }, @@ -355,12 +356,14 @@ async function detectRequirements(allLabels, prFiles, context, hasYamlLoadable) const labels = new Set(); // Check for missing tests - if ((allLabels.has('new-component') || allLabels.has('new-platform') || allLabels.has('new-feature')) && !allLabels.has('has-tests')) { + if ((allLabels.has('new-component') || allLabels.has('new-platform') || allLabels.has('new-feature') || allLabels.has('new-feature-developer')) && !allLabels.has('has-tests')) { labels.add('needs-tests'); } // Check for missing docs. - // `new-feature` (PR-body checkbox) always counts. `new-component` / `new-platform` + // `new-feature` (PR-body checkbox) always counts. `new-feature-developer` is + // deliberately excluded here: its docs live on developers.esphome.io and are + // checked separately below. `new-component` / `new-platform` // only count when at least one newly added file defines a top-level CONFIG_SCHEMA, // i.e. the new component/platform is actually loadable from YAML. const docsEligible = @@ -376,6 +379,22 @@ async function detectRequirements(allLabels, prFiles, context, hasYamlLoadable) } } + // Check for missing developer docs. `new-feature-developer` requires a + // developers.esphome.io PR link, unless every changed file outside tests/ is + // in DEV_DOCS_EXEMPT_FILES (core validators documented via docstrings only). + if (allLabels.has('new-feature-developer')) { + const prBody = context.payload.pull_request.body || ''; + const nonTestFiles = prFiles + .map(file => file.filename) + .filter(file => !file.startsWith('tests/')); + const onlyExemptFiles = nonTestFiles.every(file => DEV_DOCS_EXEMPT_FILES.includes(file)); + const hasDevDocsLink = DEVELOPER_DOCS_PR_PATTERNS.some(pattern => pattern.test(prBody)); + + if (!onlyExemptFiles && !hasDevDocsLink) { + labels.add('needs-developer-docs'); + } + } + // Check for missing CODEOWNERS if (allLabels.has('new-component')) { const codeownersModified = prFiles.some(file => diff --git a/.github/scripts/auto-label-pr/tests/detectors.test.js b/.github/scripts/auto-label-pr/tests/detectors.test.js index aab1827c44..413fdb3f94 100644 --- a/.github/scripts/auto-label-pr/tests/detectors.test.js +++ b/.github/scripts/auto-label-pr/tests/detectors.test.js @@ -1,6 +1,13 @@ const { describe, it } = require('node:test'); const assert = require('node:assert/strict'); -const { detectNewPlatforms, detectNewComponents, detectPRSize } = require('../detectors'); +const { + detectNewPlatforms, + detectNewComponents, + detectPRSize, + detectPRTemplateCheckboxes, + detectRequirements, +} = require('../detectors'); +const { MANAGED_LABELS } = require('../constants'); // Minimal GitHub API mock — only repos.getContent is called by detectNewPlatforms/detectNewComponents // to check for CONFIG_SCHEMA in newly added files. @@ -146,6 +153,125 @@ describe('detectNewComponents', () => { }); }); +// --------------------------------------------------------------------------- +// detectPRTemplateCheckboxes +// --------------------------------------------------------------------------- + +const NEW_FEATURE_LINE = '- [x] New feature (non-breaking change which adds functionality)'; +const DEV_FEATURE_LINE = '- [x] New developer-facing feature (adds functionality for component developers; no end-user configuration change)'; +const DEV_FEATURE_LINE_UNTICKED = '- [ ] New developer-facing feature (adds functionality for component developers; no end-user configuration change)'; + +function makeBodyContext(body) { + return { payload: { pull_request: { body } } }; +} + +describe('detectPRTemplateCheckboxes', () => { + it('ticked developer-facing feature checkbox adds new-feature-developer only', async () => { + const labels = await detectPRTemplateCheckboxes(makeBodyContext(DEV_FEATURE_LINE)); + assert.ok(labels.has('new-feature-developer')); + assert.ok(!labels.has('new-feature')); + }); + + it('unticked developer-facing feature checkbox adds no label', async () => { + const labels = await detectPRTemplateCheckboxes(makeBodyContext(DEV_FEATURE_LINE_UNTICKED)); + assert.ok(!labels.has('new-feature-developer')); + }); + + it('ticked new feature checkbox does not add new-feature-developer', async () => { + const labels = await detectPRTemplateCheckboxes(makeBodyContext(NEW_FEATURE_LINE)); + assert.ok(labels.has('new-feature')); + assert.ok(!labels.has('new-feature-developer')); + }); +}); + +// --------------------------------------------------------------------------- +// detectRequirements +// --------------------------------------------------------------------------- + +describe('detectRequirements', () => { + // PR body without any docs-PR link. + const NO_DOCS_CONTEXT = makeBodyContext('Just a description, no docs link.'); + const USER_DOCS_CONTEXT = makeBodyContext('Docs: esphome/esphome.io#1234'); + const DEV_DOCS_CONTEXT = makeBodyContext('Docs: esphome/developers.esphome.io#1234'); + const DEV_DOCS_URL_CONTEXT = makeBodyContext('Docs: https://github.com/esphome/developers.esphome.io/pull/1234'); + + // File sets: a normal source change vs. one confined to the exempt core validators. + const SOURCE_FILES = [ + { filename: 'esphome/components/foo/foo.py' }, + { filename: 'tests/components/foo/common.yaml' }, + ]; + const VALIDATOR_FILES = [ + { filename: 'esphome/config_validation.py' }, + { filename: 'tests/unit_tests/test_config_validation.py' }, + ]; + + it('new-feature-developer without has-tests adds needs-tests but not needs-docs', async () => { + const labels = await detectRequirements(new Set(['new-feature-developer']), SOURCE_FILES, NO_DOCS_CONTEXT, false); + assert.ok(labels.has('needs-tests')); + assert.ok(!labels.has('needs-docs')); + }); + + it('new-feature-developer with has-tests does not add needs-tests', async () => { + const labels = await detectRequirements(new Set(['new-feature-developer', 'has-tests']), SOURCE_FILES, NO_DOCS_CONTEXT, false); + assert.ok(!labels.has('needs-tests')); + }); + + it('new-feature without a docs link still adds needs-docs', async () => { + const labels = await detectRequirements(new Set(['new-feature', 'has-tests']), [], NO_DOCS_CONTEXT, false); + assert.ok(labels.has('needs-docs')); + }); + + it('new-feature-developer without a developer docs link adds needs-developer-docs', async () => { + const labels = await detectRequirements(new Set(['new-feature-developer', 'has-tests']), SOURCE_FILES, NO_DOCS_CONTEXT, false); + assert.ok(labels.has('needs-developer-docs')); + }); + + it('a developers.esphome.io shorthand link satisfies needs-developer-docs', async () => { + const labels = await detectRequirements(new Set(['new-feature-developer', 'has-tests']), SOURCE_FILES, DEV_DOCS_CONTEXT, false); + assert.ok(!labels.has('needs-developer-docs')); + }); + + it('a developers.esphome.io URL link satisfies needs-developer-docs', async () => { + const labels = await detectRequirements(new Set(['new-feature-developer', 'has-tests']), SOURCE_FILES, DEV_DOCS_URL_CONTEXT, false); + assert.ok(!labels.has('needs-developer-docs')); + }); + + it('a user docs (esphome.io) link does not satisfy needs-developer-docs', async () => { + const labels = await detectRequirements(new Set(['new-feature-developer', 'has-tests']), SOURCE_FILES, USER_DOCS_CONTEXT, false); + assert.ok(labels.has('needs-developer-docs')); + }); + + it('a developer docs link does not satisfy needs-docs for new-feature', async () => { + const labels = await detectRequirements(new Set(['new-feature', 'has-tests']), [], DEV_DOCS_CONTEXT, false); + assert.ok(labels.has('needs-docs')); + }); + + it('changes confined to core validator files are exempt from needs-developer-docs', async () => { + const labels = await detectRequirements(new Set(['new-feature-developer', 'has-tests']), VALIDATOR_FILES, NO_DOCS_CONTEXT, false); + assert.ok(!labels.has('needs-developer-docs')); + }); + + it('validator changes mixed with other source files are not exempt', async () => { + const prFiles = [...VALIDATOR_FILES, { filename: 'esphome/components/foo/foo.py' }]; + const labels = await detectRequirements(new Set(['new-feature-developer', 'has-tests']), prFiles, NO_DOCS_CONTEXT, false); + assert.ok(labels.has('needs-developer-docs')); + }); +}); + +// --------------------------------------------------------------------------- +// MANAGED_LABELS +// --------------------------------------------------------------------------- + +describe('MANAGED_LABELS', () => { + it('includes new-feature-developer so the workflow syncs it', () => { + assert.ok(MANAGED_LABELS.includes('new-feature-developer')); + }); + + it('includes needs-developer-docs so the workflow syncs it', () => { + assert.ok(MANAGED_LABELS.includes('needs-developer-docs')); + }); +}); + // --------------------------------------------------------------------------- // detectPRSize // --------------------------------------------------------------------------- diff --git a/.github/workflows/status-check-labels.yml b/.github/workflows/status-check-labels.yml index d27cc0cbec..72987c25b1 100644 --- a/.github/workflows/status-check-labels.yml +++ b/.github/workflows/status-check-labels.yml @@ -5,7 +5,7 @@ on: types: [opened, reopened, labeled, unlabeled, synchronize] permissions: - pull-requests: read # issues.listLabelsOnIssue to detect blocking labels (needs-docs, merge-after-release, chained-pr) + pull-requests: read # issues.listLabelsOnIssue to detect blocking labels (needs-docs, needs-developer-docs, merge-after-release, chained-pr) concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number }} @@ -20,7 +20,7 @@ jobs: uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: script: | - const blockingLabels = ['needs-docs', 'merge-after-release', 'chained-pr']; + const blockingLabels = ['needs-docs', 'needs-developer-docs', 'merge-after-release', 'chained-pr']; const { data: labels } = await github.rest.issues.listLabelsOnIssue({ owner: context.repo.owner, repo: context.repo.repo,