🔥 remove dedicated MLIR option for reusable-change-detection.yml and merge with C++ checks#300
Conversation
…merge with C++ checks Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughRemoved the dedicated Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-10-11T13:21:15.212ZApplied to files:
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/reusable-change-detection.yml (2)
28-30: Critical: Remove orphanedrun-mlirworkflow output.The workflow-level output
run-mlir(lines 28-30) referencesjobs.change-detection.outputs.run-mlir, but this job output is no longer defined in thechange-detectionjob outputs section (lines 44-48). This inconsistency will cause therun-mliroutput to always evaluate tofalse, breaking any consumers that depend on it.Since the PR objective is to remove the dedicated MLIR output, lines 28-30 should be deleted entirely.
🔥 Proposed fix
run-cd: description: Whether to run the continuous deployment job value: ${{ jobs.change-detection.outputs.run-cd || false }} - run-mlir: - description: Whether to run the MLIR tests - value: ${{ jobs.change-detection.outputs.run-mlir || false }}
84-95: Remove duplicatemlir/**entry in the C++ linter filter.The
mlir/**path appears twice in the file filter: once at line 88 (newly added) and again at line 92 (existing). This duplication is redundant and should be cleaned up by removing line 92.♻️ Proposed fix
with: filter: | bindings/** include/** mlir/** src/** test/** !test/python/** - mlir/** .clang-tidy .github/workflows/reusable-cpp-linter.yml .github/workflows/ci.yml
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/reusable-change-detection.ymlCHANGELOG.mdUPGRADING.md
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1360
File: .github/workflows/reusable-mlir-tests.yml:40-43
Timestamp: 2025-12-05T17:45:37.602Z
Learning: In the munich-quantum-toolkit/core repository, patch releases of LLVM dependencies don't require documentation updates, changelog entries, or additional tests beyond what's validated by passing CI checks.
Learnt from: burgholzer
Repo: munich-quantum-toolkit/yaqs PR: 212
File: CHANGELOG.md:12-15
Timestamp: 2025-10-14T14:37:38.047Z
Learning: In the munich-quantum-toolkit/yaqs project, changelog entries follow the template: "- $TITLE ([#$NUMBER]($URL)) ([**AUTHOR**](https://github.com/$AUTHOR))". Issue references should not be included in changelog entries; the PR number is sufficient for traceability.
Learnt from: denialhaag
Repo: munich-quantum-toolkit/ddsim PR: 674
File: .github/workflows/cd.yml:56-56
Timestamp: 2025-10-11T13:21:15.212Z
Learning: In the ddsim repository, modifying .github/workflows/cd.yml intentionally triggers CD workflow testing in CI through a change-detection mechanism in ci.yml. The change-detection job outputs a run-cd flag that controls whether build-sdist and build-wheel jobs execute in CI, allowing testing of the CD workflow (including wheel building and testing) before actual releases.
📚 Learning: 2025-12-05T17:45:37.602Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1360
File: .github/workflows/reusable-mlir-tests.yml:40-43
Timestamp: 2025-12-05T17:45:37.602Z
Learning: In the munich-quantum-toolkit/core repository, patch releases of LLVM dependencies don't require documentation updates, changelog entries, or additional tests beyond what's validated by passing CI checks.
Applied to files:
UPGRADING.md.github/workflows/reusable-change-detection.ymlCHANGELOG.md
📚 Learning: 2025-12-01T11:00:40.342Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 1
File: CHANGELOG.md:18-18
Timestamp: 2025-12-01T11:00:40.342Z
Learning: In the munich-quantum-toolkit/core-plugins-catalyst repository, the CHANGELOG.md intentionally references the parent MQT Core repository's release notes (https://github.com/munich-quantum-toolkit/core/releases) because the plugin repository is based on work performed in the parent repository.
Applied to files:
UPGRADING.md
📚 Learning: 2026-01-04T21:41:30.252Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1356
File: cmake/SetupMLIR.cmake:32-33
Timestamp: 2026-01-04T21:41:30.252Z
Learning: In the munich-quantum-toolkit/core repository, the project provides its own pre-built LLVM/MLIR distributions that are compiled with LLVM_ENABLE_RTTI=ON and LLVM_ENABLE_EH=ON. Users are directed to these distributions via the upgrade guide and installation instructions to avoid ABI compatibility issues. The mid-term goal is to eliminate RTTI and exception handling dependencies entirely.
Applied to files:
UPGRADING.md
📚 Learning: 2025-10-11T13:21:15.212Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/ddsim PR: 674
File: .github/workflows/cd.yml:56-56
Timestamp: 2025-10-11T13:21:15.212Z
Learning: In the ddsim repository, modifying .github/workflows/cd.yml intentionally triggers CD workflow testing in CI through a change-detection mechanism in ci.yml. The change-detection job outputs a run-cd flag that controls whether build-sdist and build-wheel jobs execute in CI, allowing testing of the CD workflow (including wheel building and testing) before actual releases.
Applied to files:
.github/workflows/reusable-change-detection.yml
📚 Learning: 2025-10-14T14:37:38.047Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/yaqs PR: 212
File: CHANGELOG.md:12-15
Timestamp: 2025-10-14T14:37:38.047Z
Learning: In the munich-quantum-toolkit/yaqs project, changelog entries follow the template: "- $TITLE ([#$NUMBER]($URL)) ([**AUTHOR**](https://github.com/$AUTHOR))". Issue references should not be included in changelog entries; the PR number is sufficient for traceability.
Applied to files:
CHANGELOG.md
🔇 Additional comments (3)
CHANGELOG.md (1)
12-18: LGTM! Changelog entries follow project conventions.The changelog entries are properly structured and follow the project's template format. Both the "Changed" and "Removed" sections accurately document the consolidation of MLIR detection into C++ workflows.
Also applies to: 271-271
.github/workflows/reusable-change-detection.yml (1)
60-60: LGTM! MLIR detection properly consolidated with C++ filters.Adding
mlir/**to both the C++ tests and C++ linter file filters correctly implements the consolidation described in the PR objectives.Also applies to: 88-88
UPGRADING.md (1)
7-12: LGTM! Clear upgrade documentation.The upgrade note clearly explains the removal of the
run-mliroutput, the rationale behind the change, and its limited scope to the MQT Core repository.
Signed-off-by: burgholzer <burgholzer@me.com>
1 participant
Description
This PR removes the dedicated
run-mliroutput from the change detection workflow and merges it with the C++ workflow checks.This is in anticipation for munich-quantum-toolkit/core#1356, which enables MLIR by default (at least for C++).
Checklist: