mirror of
https://github.com/esphome/esphome.git
synced 2026-08-17 10:52:56 +08:00
[github] Add developer-facing feature PR classification (#17795)
This commit is contained in:
@@ -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#<esphome.io PR number goes here>
|
||||
|
||||
**Pull request in [developers.esphome.io](https://github.com/esphome/developers.esphome.io) with developer documentation (if applicable):**
|
||||
|
||||
- esphome/developers.esphome.io#<developers.esphome.io PR number goes here>
|
||||
|
||||
## Test Environment
|
||||
|
||||
- [ ] ESP32
|
||||
|
||||
@@ -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'
|
||||
]
|
||||
};
|
||||
|
||||
@@ -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 =>
|
||||
|
||||
@@ -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
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user