ci: enforce EditorConfig across repository - #18761
ci: enforce EditorConfig across repository#18761Tobias Brick (tobiasb-ms) wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The updated hook configuration should restrict to types: [text] to match the stated intent and avoid scanning binaries, and the PR description indicates existing violations are still outstanding.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the repository’s pre-commit linting configuration to enforce EditorConfig rules more broadly, while carving out documented exclusions for generated or externally managed content.
Changes:
- Expand
editorconfig-checkercoverage from only*.tomlto (intended) all tracked text files. - Add explicit, documented exclusions for generated specs and specific legal/centrally managed/upstream-artifact files.
File summaries
| File | Description |
|---|---|
| .pre-commit-config.yaml | Broadens EditorConfig checking scope and documents path-based exclusions. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
.pipelines/CodeQL/CodeQL.yml is not valid YAML due to incorrect indentation under jobs:.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/12 changed files
- Comments generated: 3
- Review effort level: Lite
0d22853 to
0fa6655
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The changes are consistent with the stated goal (repo-wide EditorConfig enforcement + cleanup) and are limited to formatting/regen artifacts without introducing behavioral modifications.
Review details
- Files reviewed: 5/16 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
/azp run |
|
Azure Pipelines: 2 pipeline(s) were filtered out due to trigger conditions. |
0fa6655 to
7b7cfaa
Compare
7b7cfaa to
7ad0fc4
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
It includes an uncalled-out functional change to the kernel-doc subpackage contents (removing kernel.changelog.xz), which needs explicit intent/validation or to be split/reverted.
Review details
Suppressed comments (1)
base/comps/kernel/kernel.spec:863
- This removes Source2 and the installation/packaging of
kernel.changelog.xz, which changes the contents of thekernel-docsubpackage (not just formatting). If this is intentional, it should be called out explicitly in the PR description and validated like any other RPM output change (build + smoke-test); if it’s not intentional, please revert these deletions so the shipped documentation remains unchanged.
Source0: kernel-%{specrpmversion}.%{kextraversion}.tar.gz
Source1: Makefile.rhelver
Source10: redhatsecurebootca5.cer
Source13: redhatsecureboot501.cer
- Files reviewed: 8/24 changed files
- Comments generated: 0 new
- Review effort level: Lite
7ad0fc4 to
54be5ef
Compare
Check all tracked text files while excluding generated, legal, and centrally managed artifacts through editorconfig-checker native configuration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Format maintained files for repository-wide EditorConfig enforcement and remove the obsolete kernel changelog source and packaging path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
54be5ef to
3a5ca05
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The kernel payload change leaves its manual release unchanged and lacks required build and mock validation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 8/24 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The kernel payload changes need a manual release increment and the required build, inspection, and smoke-test validation.
Review details
Suppressed comments (2)
base/comps/kernel/kernel.spec:3229
- This removal changes the
kernel-docRPM payload, but the PR validation reports only pre-commit. Repository policy requires local-spec/source changes that affect RPM output to be built, inspected, and smoke-tested (AGENTS.md:14,24-30); please add the kernel build/package validation results, or document why a required step could not be performed and what was verified instead.
tar -h -f - --exclude=man --exclude='.*' -c Documentation | tar xf - -C $docdir
base/comps/kernel/kernel.spec:859
- This changes the kernel package contents without incrementing its manual release.
kernel.comp.toml:8-12andkernel.spec:17-20require bumpingazl_pkgreleasefor a rebuild without a version change; leaving it at 3 produces altered RPMs with the existing NEVRA, so repositories cannot reliably publish or upgrade to them. Increment the macro and re-render the generated spec.
Source1: Makefile.rhelver
- Files reviewed: 8/24 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Removing kernel.changelog.xz changes the kernel-doc payload, so use a new package release rather than publishing different contents under the existing NEVRA. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cdca3ce4-e087-4117-a5be-f57de435b9d9
There was a problem hiding this comment.
🔵 Needs a closer look
The kernel RPM content change still needs the repository-mandated build, inspection, and mock-chroot validation.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
base/comps/kernel/kernel.spec:20
- The documented validation only runs pre-commit, but this kernel rebuild also removes
kernel.changelog.xzfrom thekernel-docRPM.AGENTS.md:24-30requires RPM-output changes to be built, inspected, and smoke-tested in a mock chroot (or to document why a step is impossible). Please add that validation before merging.
- Files reviewed: 8/24 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Summary
Validation
pre-commit run --all-files --show-diff-on-failure