https://bugzilla.redhat.com/show_bug.cgi?id=1823151
Bug ID: 1823151 Summary: Review Request: thinkpad-tools - Tools to manage ThinkPad properties Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: dev@singhk.dev QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://raw.githubusercontent.com/devksingh4/thinkpad-tools/master/python3-t... SRPM URL: https://github.com/devksingh4/thinkpad-tools/releases/download/v0.11.0/pytho... Description: This CLI tool allows users of Lenovo ThinkPads to edit properties such as TrackPoint speed, battery status, and undervolting. Fedora Account System Username: dsingh
This is my first package and I need a sponsor. I am also the author of the upstream software.
Here's the link to a koji build for the software: https://koji.fedoraproject.org/koji/taskinfo?taskID=43256182
https://bugzilla.redhat.com/show_bug.cgi?id=1823151
Dev Singh dev@singhk.dev changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841 (FE-NEEDSPONSOR)
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=1823151
Fabian Affolter mail@fabian-affolter.ch changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mail@fabian-affolter.ch Doc Type|--- |If docs needed, set a value
--- Comment #1 from Fabian Affolter mail@fabian-affolter.ch --- Just some comments to get started...
- URL field is empty -> https://github.com/devksingh4/thinkpad-tools - Source0 could be %{pypi_source} - The descriptions should end with a period. - LICENSE file must be added to the %files section (%license LICENSE) - /etc/thinkpad-tools-persistence.sh looks misplaced, only configuration files should go under /etc
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_configuration_fi... - Systemd unit files: https://docs.fedoraproject.org/en-US/packaging-guidelines/Systemd/#_packagin... - Changelog is missing
Also, https://github.com/devksingh4/thinkpad-tools/issues/14 should be addressed.
https://bugzilla.redhat.com/show_bug.cgi?id=1823151
Dev Singh dev@singhk.dev changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(mail@fabian-affol | |ter.ch)
--- Comment #2 from Dev Singh dev@singhk.dev --- Hello Fabian, Thank you for your comments. I have addressed all of them, however, I believe that the persistence bash script is in the correct place. While it is a bash script, it's really a config file for the user to specify what settings should be applied on startup. Do you have another location in mind for this file?
https://bugzilla.redhat.com/show_bug.cgi?id=1823151
--- Comment #3 from Dev Singh dev@singhk.dev --- I have also added the package to a COPR for Fedora/CentOS here: https://copr.fedorainfracloud.org/coprs/dsingh/thinkpad-tools/
https://bugzilla.redhat.com/show_bug.cgi?id=1823151
Matthew Miller mattdm@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mattdm@redhat.com
--- Comment #4 from Matthew Miller mattdm@redhat.com --- Mightn't it be better to use a proper config file syntax for the persistent settings rather than a bash script? That's really an upstream issue rather than something for package review, though. (Also, along the same lines: thinkpad-tools.service should maybe be `oneshot` rather than `simple`, since it's not a daemon.) I'll file these upstream.
https://bugzilla.redhat.com/show_bug.cgi?id=1823151
Fabian Affolter mail@fabian-affolter.ch changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(mail@fabian-affol | |ter.ch) |
--- Comment #5 from Fabian Affolter mail@fabian-affolter.ch --- Removing "needinfo"...
https://bugzilla.redhat.com/show_bug.cgi?id=1823151
Robert-André Mauchin zebob.m@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |zebob.m@gmail.com
--- Comment #6 from Robert-André Mauchin zebob.m@gmail.com --- - Missing systemD scriptlets: https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_syste...
- /usr/lib/systemd/system/thinkpad-tools.service → %{_unitdir}/thinkpad-tools.service
- Separate your changelog entries by a new line
- Since you provide a binary, consider instead naming your package "thinkpad-tools" with no Python3 subpackage
- Up to upstream, but generally configuration files are named *.conf not *.ini.
https://bugzilla.redhat.com/show_bug.cgi?id=1823151
Dev Singh dev@devksingh.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution|--- |WONTFIX Last Closed| |2021-07-17 13:01:44
Product: Fedora Version: rawhide Component: Package Review
Dev Singh dev@devksingh.com has canceled Package Review package-review@lists.fedoraproject.org's request for Dev Singh dev@devksingh.com's needinfo: Bug 1823151: Review Request: thinkpad-tools - Tools to manage ThinkPad properties https://bugzilla.redhat.com/show_bug.cgi?id=1823151
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com