Split tainted Repository install out, so it can be installed if RPM F…#1343
Open
technicks89 wants to merge 4 commits into
Open
Split tainted Repository install out, so it can be installed if RPM F…#1343technicks89 wants to merge 4 commits into
technicks89 wants to merge 4 commits into
Conversation
…usion is already installed
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the Fedora RPM Fusion setup script to separate installing the “tainted” RPM Fusion repositories from the base RPM Fusion repositories, so users can add tainted repos even when RPM Fusion is already installed (addressing issue #1259).
Changes:
- Split tainted repository installation into a dedicated
installRPMFusionTainted()function invoked afterinstallRPMFusion(). - Adjusted the base RPM Fusion install flow to emit an “already installed” message when repo files are present.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Fedora RPM Fusion setup caches the Fedora version, separates standard and tainted repository handling, and invokes both flows after environment validation. ChangesRPM Fusion setup
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant User
participant RPMFusionSetup
participant RPM
User->>RPMFusionSetup: Run RPM Fusion setup
RPMFusionSetup->>RPM: Read Fedora version
RPMFusionSetup->>RPM: Install standard repositories
RPMFusionSetup->>RPMFusionSetup: Check tainted repository packages
RPMFusionSetup->>User: Prompt for tainted repositories
User->>RPMFusionSetup: Approve installation
RPMFusionSetup->>RPM: Install and enable tainted repositories
🚥 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.
🧹 Nitpick comments (1)
core/tabs/system-setup/fedora/rpm-fusion-setup.sh (1)
6-16: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueReuse
FEDORA_VERSIONin the RPM Fusion URLs
The URLs still callrpm -E %fedoraeven thoughFEDORA_VERSIONis already computed above; use the cached value there too.🤖 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 `@core/tabs/system-setup/fedora/rpm-fusion-setup.sh` around lines 6 - 16, Update installRPMFusion to interpolate the existing FEDORA_VERSION variable in both RPM Fusion release URLs instead of invoking rpm -E %fedora again; leave the repository checks and installation flow unchanged.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@core/tabs/system-setup/fedora/rpm-fusion-setup.sh`:
- Around line 6-16: Update installRPMFusion to interpolate the existing
FEDORA_VERSION variable in both RPM Fusion release URLs instead of invoking rpm
-E %fedora again; leave the repository checks and installation flow unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dd2e8689-69f9-4e36-9c57-e67502405c02
📒 Files selected for processing (1)
core/tabs/system-setup/fedora/rpm-fusion-setup.sh
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.
…usion is already installed
Type of Change
Description
Split Tainted Repo install from RPM Fusion to allow users to install tainted if they already have Fusion installed
Issues / other PRs related
Screenshots (if applicable)