Details
- Reviewers
JoshStrobl DataDrake - Group Reviewers
Triage Team
Launches without errors. Correctly displays information about CPU and RAM. Policykit authentication dialog works fine. Ran benchmark and 30 second stress test, everything worked fine.
Diff Detail
- Branch
- master
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
That's not how you remove extra unnecessary deps and "n/a" isn't a valid test plan. It is very much applicable.
Those packages are commented out right now because they don't exist in the repos yet. Currently I cannot test it because it crashes on start due to missing dependencies. Before this package is submitted those dependencies need to be packaged into the repos.
Those packages are commented out right now because they don't exist in the repos yet.
That's why we have solbuild local repositories: https://getsol.us/articles/packaging/local-repository/en/
Two of the rundeps are already in repository:
python-xdg should be pyxdg
python-yaml should be pyyaml
@algent Is there any particular reason for this naming scheme? It seems rather inconsistent.
This kind of discussion is better to be done in our irc channel #Solus-Dev.
About pyxdg this is how its developer named it. The same with pyyaml.
You can see in repology.org how other distros have named these packages: pyyaml and pyxdg.
Also GtkStressTesting is named gst from its developer. Other distros followed also same naming, just gst.
I think it is better for solus too, to use name 'gst' instead of gtkstresstesting.
Changes didn't address my comments. Have further review items as well.
| package.yml | ||
|---|---|---|
| 7 | Not a valid SPDX 3.x identifier: https://getsol.us/articles/packaging/packaging-practices/en/#licenses | |
| pspec_x86_64.xml | ||
| 23 | This is going to conflict with any usage of the oh-my-zsh git plugin. Their meson.build for the gst binary should be patched to have binary be outputted as something like gtk-stress-testing. Their service and desktop files will also likely need to be patched for the changed binary name: https://gitlab.com/leinardi/gst/-/tree/release/data | |
@JoshStrobl So should the packagename and everything be changed too, or just the binary?
| package.yml | ||
|---|---|---|
| 2 | gst is an alias for gstreamer, name should be gtk-stress-testing | |
Name, binary, data (desktop files, service files, etc.). The site-packages could probably stay the same.
Also @YakoYakoYokuYoku, please leave the reviewing the Core Team and Global Maintainers. Thanks.
Uncomment dependencies (all dependencies have been packaged and submitted for review)
For some reason GtkStressTesting is saying that it's 0.6.1 even though the package.yml clearly states 0.7.1
[Info] Building gtk-stress-testing-0.7.1 [Build] Building native package [Source] Extracting source [Build] Running step: build [Build] Enabling ccache + cd /home/build/YPKG/root/gtk-stress-testing/build/gst-0.6.1-4179e7af494ef457a30e3a7c5af5f0868afea992
What?
Otherwise LGTM. Sorry for the delay in review, I was very pre-occupied with GNOME Stack upgrade.
| package.yml | ||
|---|---|---|
| 27 | Patch and configure should go in setup step. | |
Sorry for the nonresponse, and sorry for abandoning my patch after you guys spent a lot of your time teaching me about Solus packaging. I'm on Gentoo right now, with no intention of going back to Solus. I still believe Solus is the best distribution but I want to use the Sway window manager without a desktop environment and it's a bit harder to do that on Solus. I will try to set up a virtual machine to continue contributing to Solus, but I'm very busy with other stuff at the moment.
I'm on Gentoo right now, with no intention of going back to Solus.
In that case we don't really have a reason to merge this. The goal is to have dedicated package maintainers that actively use Solus and ensure the package is well integrated. Closing.
@JoshStrobl Sorry about that, I'm back on Solus due to Gentoo being extremely troublesome :)
Closing since user is opting to go and create their own Linux distribution. I don't see a reason further support should be provided given that intent.