validators: name the exact patch headers that are missing - #95
Open
BKPepe wants to merge 1 commit into
Open
Conversation
BKPepe
force-pushed
the
fix/patch-header-envelope
branch
from
August 6, 2026 07:17
1616dff to
0f3df79
Compare
BKPepe
changed the title
BKPepe
marked this pull request as draft
August 6, 2026 07:34
BKPepe
force-pushed
the
fix/patch-header-envelope
branch
from
August 7, 2026 08:05
0f3df79 to
324b14b
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Header removals from existing patches are incorrectly classified as inherited non-blocking violations.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Balanced
Comment on lines
+1006
to
+1008
| if (reportPatches.warnings.length > 0) { | ||
| allPrWarnings.push(`**Commit [${sha.slice(0, 7)}](${html_url})** - *${commitSubject}*:\n` + reportPatches.warnings.map(w => `- ⚠️ ${w}`).join("\n")); | ||
| reportPatches.warnings.forEach(w => { patchesOutputText += ` ⚠️ Warning: ${w}\n`; }); |
Comment on lines
+1194
to
+1198
| // Roughly a third of the patches already in the OpenWrt trees predate this | ||
| // convention - in the packages feed most of them do. Editing one of those | ||
| // inherits a problem the author did not create, so say so without blocking | ||
| // the change; only patches added by this pull request are held to the rule. | ||
| return { warning: `Embedded patch file '${patchFile}' is missing ${missing.join(', ')}. It predates this pull request, so this does not block it. ${howToFix}` }; |
BKPepe
force-pushed
the
fix/patch-header-envelope
branch
3 times, most recently
from
September 7, 2026 21:04
0f75c54 to
a990b6a
Compare
BKPepe
marked this pull request as ready for review
September 8, 2026 10:01
The embedded-patch check reported every violation with the same list of four headers, even when only one was missing - to the author of an otherwise valid patch that read as a false claim - and it judged patches the pull request merely edited by the same standard. Report only the headers that are actually absent and say how to fix them: regenerate the patch with 'git format-patch'. The mbox separator line stays required, because tree patches are also applied out of tree, where several of them are concatenated into a single mbox stream and the separator is the only thing keeping them apart. 'Date:' is no longer demanded: git am fills it in and it has no effect on applicability. Only patches added by the pull request fail the check. Of the 6755 patches in the openwrt, packages, luci, routing, telephony and video trees, 64.3 % carry the full header, 27.8 % are bare quilt diffs and 7.6 % have From:/Subject: without the envelope - so editing one of the older patches now warns instead of blocking a change whose author did not introduce the problem. That leniency stops where the pull request is the cause. The file is read after the change, so a header missing now may be old news or may have just been deleted; the diff tells the two apart, and a removal this change makes is an error like any other. A check that passes with warnings also no longer summarises itself as "all patch files contain correct Git tracking headers". Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BKPepe
force-pushed
the
fix/patch-header-envelope
branch
from
September 9, 2026 00:49
a990b6a to
6b3a15e
Compare
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
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.
Fixes #76
The embedded-patch check reported every violation with the same list of four headers, even when only one was missing — to the author of an otherwise valid patch (openwrt/openwrt#23123) that read as a false claim.
Report only the headers that are actually absent and say how to fix them: regenerate the patch with
git format-patch. The mbox separator line stays required — tree patches are also applied out of tree, where several of them are concatenated into a single mbox stream forgit amand the separator is the only thing keeping them apart.Date:is no longer demanded:git amfills it in and it has no effect on applicability.