ci: retire redundant legacy GUI validation - #52
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughChangesThe CI workflows add concurrency controls, skip redundant pull-request GUI and compatibility builds, preserve selected validation paths, streamline noGUI packaging to Linux, update Windows packaging, and document the resulting workflow status and follow-up tasks. CI streamlining
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/build.yml (1)
91-107: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winMove the failure diagnostics after the clean-container test. The clean-container step runs after
Summarize build failureandUpload Linux failure logs, so a failure there won’t be captured by theseif: failure()handlers. Move them below that test or add equivalent diagnostics afterward.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build.yml around lines 91 - 107, The failure-diagnostic steps currently precede the clean-container test, so failures from that test are not reported or uploaded. Move “Summarize build failure” and “Upload Linux failure logs” to after the clean-container step, preserving their existing failure conditions and behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build.yml:
- Line 5: Update the push.branches filter in the workflow configuration to
include main alongside official-build and upstream-tracking, ensuring pushes to
main trigger the workflow.
- Around line 176-177: Update the checksum generation step after copying the
release artifact so SHA256SUMS.txt is created from within the release-assets
directory, recording asset basenames rather than release-assets/ path prefixes
while preserving the existing manifest output location.
---
Outside diff comments:
In @.github/workflows/build.yml:
- Around line 91-107: The failure-diagnostic steps currently precede the
clean-container test, so failures from that test are not reported or uploaded.
Move “Summarize build failure” and “Upload Linux failure logs” to after the
clean-container step, preserving their existing failure conditions and behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c5c5bd92-c7f9-43cf-a5ac-bacfe0de42c4
📒 Files selected for processing (3)
.github/workflows/build.ymldocs/matterviz-spectrum-log.mddocs/matterviz-spectrum-todo.md
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/matterviz-spectrum-log.md
- docs/matterviz-spectrum-todo.md
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
1 participant
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.
Summary
mainpushesv*-nogui.*release Linux-only on the Rocky Linux 8/glibc 2.28 baselinemainpush reruns so the expensive lifecycle is PR validation followed by trusted release-tag rebuildingWhy
MatterViz is the supported cross-platform distribution and its Windows/macOS packages already exercise command-line startup and analysis. A second platform-specific noGUI compilation repeats the same Multiwfn calculation core without providing a distinct supported runtime. Linux retains a separate headless/noGUI package until a WebKit-independent MatterViz Host is available.
The active
mainruleset still requires three legacy GUI contexts, Rocky compatibility, and three historical noGUI contexts. Deleting their PR triggers immediately would strand open pull requests. This change keeps those exact names as cheap compatibility gates while performing only the relevant Linux noGUI build.MatterViz workflow-internal artifact reuse and removal of the historical
feature/matterviz-guipush trigger remain deferred until PR #51 merges, because that PR owns.github/workflows/matterviz-gui.ymland adds the preview/formal updater distinction.Event model
main: no heavyweight legacy GUI or noGUI rebuildv*-nogui.*tag: fresh Linux noGUI package/test and same-run Linux release publicationVerification
actionlint1.7.12 on all changed workflowspython3 -m unittest -v tests.test_matterviz_build_names(18/18)git diff --checkSummary by CodeRabbit
Improvements
Documentation