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

Carl George 🤠 <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[email protected]
              Flags|                            |fedora-review?
             Status|NEW                         |ASSIGNED
           Assignee|[email protected]    |[email protected]



--- Comment #2 from Carl George 🤠 <[email protected]> ---
The spec URL is a 404 so I can't run fedora-review on this bug, but the SRPM
URL does work and I downloaded that to take a look.  Here are some first pass
improvements.

================================================================================

GNOME extension schema files shouldn't be compiled in the spec file, but rather
by glib2's file triggers.

    ❯ rpm -q glib2 --filetriggers | grep -1 glib-compile-schemas
    transfiletriggerin scriptlet (using /bin/sh) -- /usr/share/glib-2.0/schemas
    glib-compile-schemas /usr/share/glib-2.0/schemas &> /dev/null || :
    transfiletriggerpostun scriptlet (using /bin/sh) --
/usr/share/glib-2.0/schemas
    glib-compile-schemas /usr/share/glib-2.0/schemas &> /dev/null || :

Installing the XML schema file in /usr/share/glib-2.0/schemas like you already
are will result in it getting compiled into
/usr/share/glib-2.0/schemas/gschemas.compiled during installation.  This also
avoids the need for a direct build requirement on glib2.

    -BuildRequires:  glib2

    -glib-compile-schemas tiling-assistant@leleat-on-github/schemas

================================================================================

The minimum GNOME shell version specified in metadata.json is 48, so ideally
that can be enforced in the spec file as well.

    -Requires:       gnome-shell
    +Requires:       gnome-shell >= 48

Setting an upper limit is also possible, but I don't recommend that.  I
explained why in a recent pull request to the packaging guidelines if you're
curious.

https://forge.fedoraproject.org/packaging/guidelines/pulls/1425#issuecomment-626582

This isn't currently a requirement of the guidelines, so consider it optional.

================================================================================

Fedora Workstation out of the box doesn't have a graphical method for enabling
extensions.  A good way to handle this is by recommending the extensions app. 
This one is also optional.

    +Recommends:     gnome-extensions-app

================================================================================

Upstream has a changelog file, which ideally should be included as a doc file. 
This one is also optional.

    +%doc CHANGELOG.md

================================================================================

There is a missed spot where you can use the uuid macro, which I think improves
the readability.  This one is also optional.

    -    msgfmt -c "$FILE" -o
"locale/$LANG/LC_MESSAGES/[email protected]"
    +    msgfmt -c "$FILE" -o "locale/$LANG/LC_MESSAGES/%{uuid}.mo"


-- 
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
https://bugzilla.redhat.com/show_bug.cgi?id=2476900

Report this comment as SPAM: 
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202476900%23c2

-- 
_______________________________________________
package-review mailing list -- [email protected]
To unsubscribe send an email to [email protected]
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/[email protected]
Do not reply to spam, report it: 
https://forge.fedoraproject.org/infra/tickets/issues/new

Reply via email to