Conversation
Instead of using autologin with shrc to run the setup, implement a cleaner rc.d service that handles the setup-to-lightdm transition. The setup app now exits cleanly via Gtk.main_quit() and lets the rc.d service continue boot. - Bump version from 0.1 to 0.3 - Add ghostbsd_setup rc.d script installation - Replace direct lightdm startup with Gtk.main_quit() - Remove start_lightdm import and call
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReplaces the previous autologin-based setup completion flow with an rc.d service–driven transition to lightdm, cleaning up the GTK exit path and installing the new service script, while bumping the application version. Sequence diagram for rc.d-driven setup completion and GTK exitsequenceDiagram
participant RcD_initial_setup
participant SetupStationApp
participant GtkMainLoop
RcD_initial_setup->>SetupStationApp: setup_system
SetupStationApp->>SetupStationApp: enable_lightdm
SetupStationApp->>GtkMainLoop: Gtk.main_quit
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="setup.py" line_range="145-146" />
<code_context>
(f'{prefix}/lib/setup-station', ['src/ghostbsd-style.css']),
(f'{prefix}/lib/setup-station/image', lib_setup_station_image),
- (f'{prefix}/share/applications', ['src/setup-station.desktop'])
+ (f'{prefix}/share/applications', ['src/setup-station.desktop']),
+ (f'{prefix}/etc/rc.d', ['src/initial_setup'])
]
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Installing the rc.d script under `prefix/etc/rc.d` may not match system expectations for startup scripts.
Using `f'{prefix}/etc/rc.d'` ties the script location to the Python install prefix (e.g., virtualenv) instead of the system-wide `/etc/rc.d`, so the service may not be discovered or managed by the OS. It would be better to use the canonical rc.d path for the target platform (e.g. `/etc/rc.d` or `${LOCALBASE}/etc/rc.d`), or make this path configurable to align with packaging conventions.
Suggested implementation:
```python
(os.environ.get('RC_D_DIR', '/etc/rc.d'), ['src/initial_setup'])
```
1. Ensure `os` is imported at the top of `setup.py` if it is not already:
- Add `import os` alongside the existing imports (e.g. near `import sys`).
2. If your target platform prefers a different default (e.g. `${LOCALBASE}/etc/rc.d`), adjust the default accordingly, for example:
- `os.environ.get('RC_D_DIR', '/usr/local/etc/rc.d')`
3. If you have packaging tooling (e.g. ports, pkg, or distro-specific spec files), set `RC_D_DIR` appropriately in those packaging scripts so the install path matches system policy.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Instead of using autologin with shrc to run the setup, implement a cleaner
rc.d service that handles the setup-to-lightdm transition. The setup app now
exits cleanly via Gtk.main_quit() and lets the rc.d service continue boot.
Summary by Sourcery
Replace the autologin-based setup completion with an rc.d service–driven transition to lightdm and update the package version accordingly.
New Features:
Enhancements: