Bug 2050275 - Review Request: opentoonz - 2D animation software
Summary: Review Request: opentoonz - 2D animation software
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Neal Gompa
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2022-02-03 15:36 UTC by Diego Herrera
Modified: 2022-02-11 18:25 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2022-02-11 18:25:54 UTC
Type: ---
Embargoed:
ngompa13: fedora-review+


Attachments (Terms of Use)

Description Diego Herrera 2022-02-03 15:36:48 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/dherrera/opentoonz/fedora-rawhide-x86_64/03293627-opentoonz/opentoonz.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/dherrera/opentoonz/fedora-rawhide-x86_64/03293627-opentoonz/opentoonz-1.5.0-1.fc36.src.rpm

Description: OpenToonz is a 2D animation software published by DWANGO. It is based on Toonz Studio Ghibli Version, originally developed in Italy by Digital Video, Inc., and customized by Studio Ghibli over many years of production.

Fedora Account System Username: dherrera

Comment 1 Artur Frenszek-Iwicki 2022-02-04 08:43:26 UTC
> Source0: https://github.com/opentoonz/opentoonz/archive/refs/tags/v1.5.0.tar.gz
You can use %{version} as part of the Source URL so you don't have to edit it every time.

> BuildRequires: cmake gcc gcc-c++ qt5-qtbase-devel qt5-qtsvg-devel
> BuildRequires: qt5-qtscript-devel qt5-qttools-devel qt5-qtmultimedia-devel
> BuildRequires: qt5-qtserialport-devel opencv-devel libtiff-devel libpng-devel
> ...
This is allowed, but personally I'd really encourage you to sort these alphabetically
and possibly separate them one-per-line.

> %package data
> Summary: OpenToonz data files
> BuildArch: noarch
> ...
> %package doc
> Summary: OpenToonz doc files
Shouldn't the documentation be noarch as well?

> cd redhat-linux-build
> %make_build
Does "%cmake_build" not work properly here?

> cd redhat-linux-build
> %make_install
What about "%cmake_install" here?

> desktop-file-install \
>    --dir ${RPM_BUILD_ROOT}%{_datadir}/applications \
>    --delete-original \
>    ${RPM_BUILD_ROOT}%{_datadir}/applications/io.github.OpenToonz.desktop
If I'm reading this right, this replaces the file with itself.
Instead of installing the file over itself, you can just run "desktop-file-validate" on it.

Comment 3 Neal Gompa 2022-02-05 10:52:56 UTC
> rm -rf ${RPM_BUILD_ROOT}%{_datadir}/metainfo

Why are you deleting the metainfo files? Those are used by GNOME Software to show the application in the repository.

Comment 4 Diego Herrera 2022-02-06 02:59:27 UTC
(In reply to Neal Gompa from comment #3)
> > rm -rf ${RPM_BUILD_ROOT}%{_datadir}/metainfo
> 
> Why are you deleting the metainfo files? Those are used by GNOME Software to
> show the application in the repository.

Cool, I didn't know about that, just thought that they were irrelevant artifacts.

I just found out that we actually have policies regarding those files [1], so i'll check if the ones provided by upstream apply before adding them again.

Thanks for the help!

[1] https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/

Comment 6 Neal Gompa 2022-02-10 14:26:38 UTC
Review notes:

* Packaging complies with the guidelines
* Package builds and installs
* No serious issues from rpmlint
* Licensing is correct and license files are correctly installed

PACKAGE APPROVED.

Comment 7 Gwyn Ciesla 2022-02-10 19:31:51 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/opentoonz


Note You need to log in before you can comment on or make changes to this bug.