Conversation
|
@phlexbot format |
|
No automatic markdownlint fixes were necessary. |
|
No automatic jsonnetfmt fixes were necessary. |
|
No automatic cmake-format fixes were necessary. |
|
No automatic clang-format fixes were necessary. |
|
@copilot it seems that any relevant alerts found by the checks that are run won't be uploaded due to the checks that weren't run, per https://github.com/Framework-R-D/phlex/pull/321/checks?check_run_id=63550310202. Please verify this concern, and analyze/resolve if appropriate/possible. |
There was a problem hiding this comment.
Pull request overview
This pull request adds per-language change detection to the CodeQL workflow to avoid running unnecessary language scans when only a subset of languages have changed files. The workflow analyzes three languages (C++, Python, and GitHub Actions) and on pull requests, it now detects which languages have relevant changes and only runs scans for those languages, potentially saving 35-70 minutes of CI time on typical PRs.
Changes:
- Added three detection jobs (
detect-changes-cpp,detect-changes-python,detect-changes-actions) that use thedetect-relevant-changesaction to identify changed files for each language - Added
determine-languagesjob that builds a dynamic language matrix based on detection results - Added new workflow inputs (
pr-base-sha,pr-head-sha,ref,repo) to support reusable workflow scenarios - Detection is skipped for scheduled runs, manual triggers, pushes to main/develop, and when
language-matrixis explicitly provided - Updated documentation to describe the new detection behavior
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
.github/workflows/codeql-analysis.yaml |
Implements per-language detection with new jobs, adds workflow inputs for reusable workflow support, updates job dependencies and matrix building |
.github/REUSABLE_WORKFLOWS.md |
Documents the new inputs, detection behavior, and time savings for the CodeQL workflow |
.github/CodeQL-README.md |
Adds detailed documentation about language-specific analysis and detection behavior by event type |
Verified the concern - this is an expected informational warning that doesn't prevent alerts from being uploaded. What's happening:
Impact:
Analysis: Trade-off:
Recommendation: Accept the warning. The benefits far outweigh the cosmetic inconvenience. This is standard behavior when optimizing CodeQL scans based on file changes. If you'd prefer to avoid the warning entirely, I can remove the detection logic and always scan all languages, but this would defeat the purpose of the optimization. |
ebab9ef to
01ee81c
Compare
knoepfel
left a comment
There was a problem hiding this comment.
Just a couple comments for your consideration, @greenc-FNAL
|
@phlexbot cmake-fix |
|
Automatic cmake-format fixes pushed (commit 923e2b3). |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #321 +/- ##
==========================================
+ Coverage 80.19% 80.33% +0.13%
==========================================
Files 126 127 +1
Lines 3070 3102 +32
Branches 547 547
==========================================
+ Hits 2462 2492 +30
Misses 381 381
- Partials 227 229 +2
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
…ons) Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
… language-matrix input Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
…pport - Remove default value from language-matrix input to enable proper detection - Update head-ref in all detect-changes jobs to use pr-head-sha pattern - Follows established pattern from python-check, cmake-build workflows Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
923e2b3 to
2999701
Compare
Cost/Benefit Analysis: Split Detection Checks for CodeQL, C++, and Python
✅ ANALYSIS COMPLETE - IMPLEMENTATION SUCCESSFUL
Executive Summary
After comprehensive analysis, splitting detection checks is HIGHLY BENEFICIAL and has been successfully implemented.
Cost/Benefit Analysis Results
Benefits (HIGH VALUE)
Costs (LOW)
Implementation Complete
Files Changed
.github/workflows/codeql-analysis.yaml(+226 lines).github/REUSABLE_WORKFLOWS.md(updated).github/CodeQL-README.md(updated)Recent Fixes (Latest Commit)
New Features
Quality Assurance
Detection Behavior
Expected Time Savings
Known Behavior
"Configurations not found" warning: When languages are skipped in a PR, GitHub shows an informational warning because it can't compare those language results against the main branch baseline. This is expected and doesn't prevent alert upload or affect security scanning. The warning is cosmetic.
Recommendation
READY TO MERGE - High value, low cost, low risk, fully tested and documented with all review feedback addressed.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.