1. I think, using "%{_builddir}/feather/redhat-linux-build/bin/feather" in %install section is not portable. Since out of source build was introduced, the directory where build happens changed at least once, and I won't be surprised if it will change in the future again. Better use "%{_vpath_builddir}/bin/feather" instead. Also, it will improve readability.
2. Or even better, try to use %cmake_install to install binary, and resort to manual installation only if this method doesn't work, but do it with "install" command, not "cp".
3. Using "%{_builddir}/feather/" prefix in %install section is unnecessary and harms readability: %install script is already run in this directory.

Please note that this is not a proper complete review, I've just listed things that I found by looking at the spec file.

Thanks for the review!

1.) Thanks, I did not know about _vpath_builddir. Very useful!
2.) Unfortunately that installs the binary to /usr/feather, installs header files at random places, installs libraries that are only used internally, etc.
3.) Thanks, removed.

I updated the spec file (no updated SRPM yet).

Would you be willing to formally review?

"Free and open-source" applies to all packages in Fedora. So that part of the Summary
is not useful. And it you don't know what "monero" is, and I don't, the rest won't tell
you much either… Please try to write the Summary+description so that it is meaningful to
someone who is completely new to the subject.

Also "written in C++ with the Qt framework" is not terribly interesting to users. You
can mention Qt, but other details of implementation are unimportant.

> sed -i 's/^\(Icon\|Exec\)=feather$/&wallet/' %{buildroot}%{_datadir}/applications/featherwallet.desktop
> sed -i 's/^Name=Feather$/& Wallet/' %{buildroot}%{_datadir}/applications/featherwallet.desktop

Those commands should be in %prep. This is where we apply patches and do other modifications
to sources.

> mkdir -p %{buildroot}%{_bindir} %{buildroot}%{_datadir}/applications %{buildroot}%{_datadir}/pixmaps
> cp %{_vpath_builddir}/bin/feather %{buildroot}%{_bindir}/featherwallet
> cp src/assets/feather.desktop %{buildroot}%{_datadir}/applications/featherwallet.desktop

It's a bit nicer to say:
install -D %{buildroot}%{_bindir}/featherwallet %{_vpath_builddir}/bin/feather
install -m0644 -D %{buildroot}%{_datadir}/applications/featherwallet.desktop src/assets/feather.desktop
etc.

I'd recommend dropping the manual Release and %changelog handling.
See https://docs.pagure.org/fedora-infra.rpmautospec/index.html.

> %global debug_package %{nil}

Why? This is usually a sign of other problem with the build systems, usually missing flags.
Please drop this and instead figure out what is needed to for the -debuginfo package to
generate properly.

Since this is a graphical application, it should have an appdata file:
https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/
(with a pretty screenshot ;))

feather-2.3.0-1.fc37.src.rpm

Sorry, last comment was in accident when changing the title.

I uploaded a new SRPM: https://nil.im/feather-2.3.0-1.fc37.src.rpm
Updated spec file: https://nil.im/feather.spec

PTAL!

> License:       BSD

The License tag must be in SPDX format:

$ license-fedora2spdx 'BSD'
Warning: more options on how to interpret BSD. Possible options: ['BSD-1-Clause', 'BSD-2-Clause-FreeBSD', 'BSD-2-Clause-Views', 'BSD-2-Clause', 'BSD-3-Clause-Modification', 'BSD-3-Clause']
{{pick BSD choice}}

> ExcludeArch: armv7hl

Please use this: ExcludeArch: %{arm} %{ix86}

> gpgv2 --quiet --keyring %{SOURCE2} %{SOURCE1} %{SOURCE0}

Please switch to %{gpgverify} macro:
%{gpgverify} --keyring='%{SOURCE2}' --signature='%{SOURCE1}' --data='%{SOURCE0}'

> -DCMAKE_BUILD_TYPE=RelWithDebInfo

You can use Release here, because `-g` flag is already present in Fedora build flags. RelWithDebInfo will just duplicate it.

> # Their install target is not meant to be used. It installs the binary into
> # /usr (not /usr/bin) and installs a bunch of other useless files.

I think you should port their CMake build manifest to GNUInstallDirs and send a pull request instead of manually calling install binary.

> The License tag must be in SPDX format:

Done

> Please use this: ExcludeArch: %{arm} %{ix86}

Done

> Please switch to %{gpgverify} macro:

Done

> You can use Release here, because `-g` flag is already present in Fedora build flags. RelWithDebInfo will just duplicate it.

This doesn't work - with just Release (the default IIRC), it'll result in an empty debug package. RelWithDebInfo makes it work.

> I think you should port their CMake build manifest to GNUInstallDirs and send a pull request instead of manually calling install binary.

That doesn't help with this release, though, so is something for later :).

> This doesn't work - with just Release (the default IIRC), it'll result in an empty debug package. RelWithDebInfo makes it work.

Then the package ignores the Fedora build flags. You need to check this.

This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time. We're sorry
it is taking so long. If you're still interested in packaging this software
into Fedora repositories, please respond to this comment clearing the
NEEDINFO flag.

You may want to update the specfile and the src.rpm to the latest version
available and to propose a review swap on Fedora devel mailing list to increase
chances to have your package reviewed. If this is your first package and you
need a sponsor, you may want to post some informal reviews. Read more at
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.

Without any reply, this request will shortly be considered abandoned
and will be closed.
Thank you for your patience.

This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.