[Bug 1878350] Review Request: gammastep - Adjusts the color temperature of your screen according to time of day

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1878350



--- Comment #2 from Aleksei Bavshin <alebastr89@xxxxxxxxx> ---
(In reply to Andy Mender from comment #1)
Thanks for the review!
Updated files:
Spec URL:
https://download.copr.fedorainfracloud.org/results/alebastr/sway-extras/fedora-rawhide-x86_64/01659102-gammastep/gammastep.spec
SRPM URL:
https://download.copr.fedorainfracloud.org/results/alebastr/sway-extras/fedora-rawhide-x86_64/01659102-gammastep/gammastep-2.0.2-0.1.fc34.src.rpm

---
> If the gammastep-indicator subpackage is a regular Python package, it should
> (if possible) provide egg-info like so:
> > %{python3_sitelib}/gammastep-indicator-%{version}-py%{python3_version}.egg-info/

Nope, it's all autotools. To my knowledge, it's not possible to generate
egg-info with automake python module.

> - systemd_user_post is invoked in %post and systemd_user_preun in %preun
>   for Systemd user units service files.
>   Note: Systemd user unit service file(s) in gammastep, gammastep-indicator
>   See: https://docs.fedoraproject.org/en-US/packaging-
>   guidelines/Scriptlets/#_user_units

I have no idea what that means. I'm pretty sure the scriptlets section adheres
to the guidelines.
Is it fedora-review bug?

>      Review: The src/gamma-control.xml file has an extra in-code legal
> disclaimer 
>      that resembles the MIT license. Please, have a look at it.

Fixed, thanks!

> [!]: License file installed when any subpackage combination is installed.
>      Review: since the gammastep-indicator subpackage does not explicitly
> Requires
>      the main gammastep package, one should add the license file to it as
> well.

Added Requires to gammastep-indicator.

> [x]: Package must own all directories that it creates.
>      Note: Directories without known owners: /usr/lib/systemd/user,
>      /usr/lib/systemd
>      Review: please, make gammastep own these dirs or add the necessary
> Requires.

That would make the package depend on `systemd` or own systemd unit
directories.
Both are wrong things to do for a simple package.

> [x]: Sources can be downloaded from URI in Source: tag
>      Note: Could not download Source0:
>     
> https://gitlab.com/chinstrap/gammastep/-/archive/v2.0.2/gammastep-v2.0.2.tar.
> gz
>      See: https://docs.fedoraproject.org/en-US/packaging-
>      guidelines/SourceURL/
>      Review: Some route redirection issue in fedora-review? 
>      The URL is perfectly accessible.
fedora-review seems to have trouble with any gitlab URLs :(

>      Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
>      gammastep-indicator
>      Review: Is the gammastep-indicator subpackage an independent subpackage?

No, gammastep-indicator should require the main package. Thanks for catching
that.

> gammastep-indicator.x86_64: E: explicit-lib-dependency libappindicator-gtk3

Please, ignore this rpmlint error.
There's a GObject introspection import from python file which is not supported
by rpm automatic dependency generators. Thus, explicit lib dependency.


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux