https://bugzilla.redhat.com/show_bug.cgi?id=1924660 Lubomir Rintel <lkundrak@xxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |lkundrak@xxxxx Assignee|nobody@xxxxxxxxxxxxxxxxx |lkundrak@xxxxx Doc Type|--- |If docs needed, set a value --- Comment #1 from Lubomir Rintel <lkundrak@xxxxx> --- Thank you for packaging this. Here's my review: * Package named correctly * Latest version used * SPEC file clean and legible * Correct license tag used * License good for Fedora * Sane filelist * Sane requires/provides * No scriptlets shipped * Correct compiler flags used * Builds fine in mock Below are some things I'd prefer you considered addressing before I approve the package. All of them fairly minor: 1.) Please use the official dist tarball instead of just a git archive > Source0: https://github.com/%{name}/%{name}/archive/ATARI800_%{ver_}/%{name}-%{version}.tar.gz The official source tarball seems to be autotools' make dist generated: https://github.com/atari800/atari800/releases/download/ATARI800_4_2_0/atari800-4.2.0-src.tgz > BuildRequires: gcc, autoconf, automake You don't need to drag in autoconf & automake once you use the dist tarball. > autoreconf -f -i Likewise, you don't need this. 2.) Please consider removing the author reference from description: > %description ... > Authors: > David Firth > and Atari800 Development Team (see CREDITS for a full list) While the credit might be due, %description servers a different purpose -- to inform the user what the package contains and no more than that. I believe shipping the CREDITS file is a sufficient way of letting the interested users know who wrote the software. 3.) Don't bother removing $RPM_BUILD_ROOT > %install > rm -rf $RPM_BUILD_ROOT rpmbuild does that already: Executing(%install): /bin/sh -e /var/tmp/rpm-tmp.PRXywD + umask 022 + cd /home/lkundrak/rpmbuild/BUILD + '[' /home/lkundrak/rpmbuild/BUILDROOT/atari800-4.2.0-1.fc33.x86_64 '!=' / ']' + rm -rf /home/lkundrak/rpmbuild/BUILDROOT/atari800-4.2.0-1.fc33.x86_64 ++ dirname /home/lkundrak/rpmbuild/BUILDROOT/atari800-4.2.0-1.fc33.x86_64 + mkdir -p /home/lkundrak/rpmbuild/BUILDROOT + mkdir /home/lkundrak/rpmbuild/BUILDROOT/atari800-4.2.0-1.fc33.x86_64 + cd atari800-ATARI800_4_2_0 + rm -rf /home/lkundrak/rpmbuild/BUILDROOT/atari800-4.2.0-1.fc33.x86_64 + /usr/bin/make install DESTDIR=/home/lkundrak/rpmbuild/BUILDROOT/atari800-4.2.0-1.fc33.x86_64 'INSTALL=/usr/bin/install -p' It may not have always does done, but modern versions do that, and you already depend on new enough RPM by using macros such as %make_install or %make_build. > %make_install 4.) Please s/atari800.1.gz/atari800.1*/ > %files ... > %{_mandir}/man1/atari800.1.gz In theory, RPM might be configured not to compress manuals. It's a good practice to use a wildcard here. 5. ) Please use %_pkgdocdir in place of /usr/share/doc/atari800 > %files ... > %doc /usr/share/doc/atari800/README.TXT > %doc /usr/share/doc/atari800/README Or %{_docdir}/atari800; whichever you prefer. -- 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 Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
