Bug 1924660

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.

Thank you for review. Your suggestions have been applied. Changelog:

* Thu Mar 04 2021 Ján ONDREJ (SAL) <ondrejj(at)salstar.sk> - 4.2.0-2
- Updated description, source URL.
- Removed autoconf/automake and cleaning of RPM_BUILD_ROOT.
- Updated package doc directory to macro.
- Added configure --docdir parameter to allow build for epel7 too.

Updated spec and SRPM:

Spec URL: https://www.salstar.sk/pub/fedora/SPECS/atari800.spec
SRPM URL: https://www.salstar.sk/pub/fedora/33/SRPMS/atari800-4.2.0-2.fc33.src.rpm

Looks very good now. Thank you.

APPROVED

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/atari800

FEDORA-2021-2cbcf38c7a has been submitted as an update to Fedora 33. https://bodhi.fedoraproject.org/updates/FEDORA-2021-2cbcf38c7a

FEDORA-2021-77bf5a7792 has been submitted as an update to Fedora 32. https://bodhi.fedoraproject.org/updates/FEDORA-2021-77bf5a7792

FEDORA-EPEL-2021-a5eae8e39d has been submitted as an update to Fedora EPEL 8. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-a5eae8e39d

FEDORA-EPEL-2021-26b967230c has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-26b967230c

FEDORA-2021-2cbcf38c7a has been pushed to the Fedora 33 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2021-2cbcf38c7a \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-2cbcf38c7a

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-EPEL-2021-a5eae8e39d has been pushed to the Fedora EPEL 8 testing repository.

You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-a5eae8e39d

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-EPEL-2021-26b967230c has been pushed to the Fedora EPEL 7 testing repository.

You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-26b967230c

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2021-77bf5a7792 has been pushed to the Fedora 32 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2021-77bf5a7792 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-77bf5a7792

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2021-2cbcf38c7a has been pushed to the Fedora 33 stable repository.
If problem still persists, please make note of it in this bug report.

FEDORA-2021-77bf5a7792 has been pushed to the Fedora 32 stable repository.
If problem still persists, please make note of it in this bug report.

FEDORA-EPEL-2021-a5eae8e39d has been pushed to the Fedora EPEL 8 stable repository.
If problem still persists, please make note of it in this bug report.

FEDORA-EPEL-2021-26b967230c has been pushed to the Fedora EPEL 7 stable repository.
If problem still persists, please make note of it in this bug report.