Bug 1362626

I will review it tonight

It's been a week now. Are you going to review the package any time soon, please?

> %define commit 0054ae4cc15ea715148e65d4390fa774c37bb0aa
%global commit 0054ae4cc15ea715148e65d4390fa774c37bb0aa
%global shortcommit %(c=%{commit}; echo ${c:0:7})

> Version:        0.0
Version: 0

> Release:        1.20160802git0054ae4c%{?dist}
Release: 0.1git%{shortcommit}%{?dist}

> License:        GPLv2
didn't check carefully, but I suppose GPLv2+

> Source0:        https://github.com/MartinBriza/MediaWriter/archive/%{commit}.tar.gz
Source0: %{url}/archive/%{commit}/%{name}-%{shortcommit}.tar.gz

> BuildRequires:  libappstream-glib
why would you need that? you don't install/validate any of appstream xmls.

> Requires:       udisks2
actually as part of F25 we replaced udisks2 with storaged, so probably makes sense to fix it.

> mkdir %{_target_platform}
move to %prep

> %{qmake_qt5} PREFIX=/usr ..
don't hardcode "/usr", use %{_prefix}. Also not sure if you really use it in upstream.

> make %{?_smp_mflags} -C %{_target_platform}
%make_build -C %{_target_platform}

> ls -l %{buildroot}
drop it.

> %{_bindir}/mediawriter
%{_bindir}/%{name}

> %{_libexecdir}/mediawriter/helper
then who owns %{_libexecdir}/mediawriter/ ?
just use %{_libexecdir}/%{name}/

* Missing BuildRequires:  gcc-c++
* I guess all Requires should have %{?_isa}

Hi Igor,
thank you for the thorough review.
I uploaded new (and fixed) specfile and srpms here:

Spec URL: https://mbriza.fedorapeople.org/mediawriter/2/mediawriter.spec
SRPM URL: https://mbriza.fedorapeople.org/mediawriter/2/mediawriter-0-0.1git0054ae4.fc24.src.rpm

I tended to all your notes while commenting out libappstream (will be used in the future) and still using the PREFIX variable as it actually is used by the project.

> #f24/23 Requires: udisks2               
> Requires:       storaged%{?_isa}
* whitespace detected ;)
* you can do something:
%if 0%{?fedora} && 0%{?fedora} < 25
Requires: udisks2%{?_isa}
%else
Requires: storaged%{?_isa}
%endif

> %{_libexecdir}/%{name}
I would prefer to have trailing "/" to be sure that it's directory

> %if 0%{?fedora} && 0%{?fedora} < 2

Noo, I wanted to avoid this. But since you suggested so, it's there. And the other stuff, too.

Spec URL: https://mbriza.fedorapeople.org/mediawriter/3/mediawriter.spec
SRPM URL: https://mbriza.fedorapeople.org/mediawriter/3/mediawriter-0-0.1git0054ae4.fc24.src.rpm

Thank you for the review!

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/mediawriter

mediawriter-0-1.1git728a879.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2016-3ddbdc6ebb

mediawriter-0-1.1git728a879.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-3ddbdc6ebb

mediawriter-0-1.1git728a879.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.