Spec URL: https://download.copr.fedorainfracloud.org/results/basilcrow/uwsm/fedora-43-x86_64/09970056-uwsm/uwsm.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/basilcrow/uwsm/fedora-43-x86_64/09970056-uwsm/uwsm-0.26.0-1.fc43.src.rpm Description: Universal Wayland Session Manager Fedora Account System Username: basilcrow
Taking this review.
Copr build: https://copr.fedorainfracloud.org/coprs/build/9970926 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2426865-uwsm/fedora-rawhide-x86_64/09970926-uwsm/fedora-review/review.txt Found issues: - Unversionned Python dependency found. Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_dependencies - Systemd user unit service file(s) in uwsm Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_user_units Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Initial review feedback: * The various integrations for specific window managers should be broken out into their own subpackages (similar to what was done for lxqt-wayland-session) * The scripts for integrating with various bars should similarly be broken out into their own subpackages * Systemd presets are not permitted in any package except fedora-release. This will need to be evaluated and submitted per policy: https://docs.fedoraproject.org/en-US/packaging-guidelines/DefaultServices/ These things need to be addressed before we can progress.
Hi Neal, Thanks for the review and for pointing out the systemd preset policy. The systemd service in question is fumon. It isn't essential to UWSM, can be disabled at compile time, and the preset was already disabled by default. To align cleanly with the preset policy, I've disabled it at compile time and removed it from the package entirely. As a result, this package no longer ships any presets and aligns with the policy. Updated build here: https://download.copr.fedorainfracloud.org/results/basilcrow/uwsm/fedora-rawhide-x86_64/10208356-uwsm/uwsm.spec https://download.copr.fedorainfracloud.org/results/basilcrow/uwsm/fedora-rawhide-x86_64/10208356-uwsm/uwsm-0.26.1-1.fc45.src.rpm Regarding your other comments, this review is intentionally limited to the UWSM core and does not include integrations for specific window managers. The LXQt integration is prototyped here (not packaged yet): https://github.com/basil/lxqt-uwsm-session As you suggested, I plan to submit that as a separate package review. It will depend on the UWSM core package from this review. Additional integrations (for other window managers/bars) can then be developed and packaged in the same way. Concretely, the plan is: 1. Package UWSM core 2. Package LXQt integration 3. Package additional window manager/bar integrations as needed > These things need to be addressed before we can progress. I don't see a blocker to moving forward with the UWSM core package in this review, and then handling integrations in follow-up new package reviews, one for each integration. The UWSM core package in this package review has independent value on its own, and introducing it now doesn't make later integration work harder or introduce any technical debt. Rather, it is merely the first step that enables future steps in the design we're both aligned on. Basil
Created attachment 2132903 [details] The .spec file difference from Copr build 9970926 to 10208388
Copr build: https://copr.fedorainfracloud.org/coprs/build/10208388 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2426865-uwsm/fedora-rawhide-x86_64/10208388-uwsm/fedora-review/review.txt Found issues: - Unversionned Python dependency found. Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_dependencies - Systemd user unit service file(s) in uwsm Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_user_units Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Fixed the "unversioned Python dependency" warning from Fedora Review Service. The "systemd user unit service" warning is a false positive. New build: https://download.copr.fedorainfracloud.org/results/basilcrow/uwsm/fedora-rawhide-x86_64/10208426-uwsm/uwsm.spec https://download.copr.fedorainfracloud.org/results/basilcrow/uwsm/fedora-rawhide-x86_64/10208426-uwsm/uwsm-0.26.1-1.fc45.src.rpm
Created attachment 2132904 [details] The .spec file difference from Copr build 10208388 to 10208429
Copr build: https://copr.fedorainfracloud.org/coprs/build/10208429 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2426865-uwsm/fedora-rawhide-x86_64/10208429-uwsm/fedora-review/review.txt Found issues: - Systemd user unit service file(s) in uwsm Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_user_units Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
I forgot that UWSM core also includes "plugins" for different compositors. This is probably what you were referring to above when you were asking for integrations to be split into separate packages. Yes, I agree these need to be split out and have split them in my latest commit: https://download.copr.fedorainfracloud.org/results/basilcrow/uwsm/fedora-rawhide-x86_64/10208448-uwsm/uwsm.spec https://download.copr.fedorainfracloud.org/results/basilcrow/uwsm/fedora-rawhide-x86_64/10208448-uwsm/uwsm-0.26.1-1.fc45.src.rpm
Created attachment 2132907 [details] The .spec file difference from Copr build 10208429 to 10208459
Copr build: https://copr.fedorainfracloud.org/coprs/build/10208459 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2426865-uwsm/fedora-rawhide-x86_64/10208459-uwsm/fedora-review/review.txt Found issues: - Systemd user unit service file(s) in uwsm Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_user_units Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Updating my plan from earlier: 1. Package UWSM core + plugins (this ticket - one package for UWSM core and one package for each UWSM compositor plugin) 2. Package LXQt integration - a future ticket to package https://github.com/basil/lxqt-uwsm-session once it is released 3. Package additional integrations as needed Note that user-visible sessions are part of #2 or #3, not #1. Upstream UWSM provides no sessions, but for example #2 (not part of this package review) provides sessions inhttps://github.com/basil/lxqt-uwsm-session/tree/master/wayland-sessions