Bug 1340946 - Review Request: photoqt - A fast Qt image viewer
Summary: Review Request: photoqt - A fast Qt image viewer
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jaroslav Reznik
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-05-30 19:58 UTC by Jiri Eischmann
Modified: 2020-04-23 11:58 UTC (History)
3 users (show)

Fixed In Version: photoqt-1.4-3.fc24
Clone Of:
Environment:
Last Closed: 2020-04-23 11:58:35 UTC
Type: ---
Embargoed:
jreznik: fedora-review+


Attachments (Terms of Use)

Description Jiri Eischmann 2016-05-30 19:58:23 UTC
Spec URL: https://dl.dropboxusercontent.com/u/1309518/photoqt.spec
SRPM URL: https://dl.dropboxusercontent.com/u/1309518/photoqt-1.4-1.fc24.src.rpm
Description: PhotoQt is a fast and highly configurable image viewer with a simple and nice interface.
Fedora Account System Username: eischmann

Comment 1 Jaroslav Reznik 2016-05-31 08:20:37 UTC
Name: ok
Version & release: ok
Summary: ok
License: ok (GPLv2+)
URL: ok
Sources: ok (SHA256 9df3392f0bd746778f119a4da98a32e8ae9ae215564df97d7bcf52efa4d08604)

BuildRequires: not ok

BuildRequires:  qt5-qtbase-tds
BuildRequires:  qt5-qtbase-odbc
BuildRequires:  qt5-qtbase-ibase
BuildRequires:  qt5-qtbase-mysql
BuildRequires:  qt5-qtbase-postgresql

are not BuildRequires and are not needed to build the package

Description: ok

Files: ok
Scratch build: builds ok, http://koji.fedoraproject.org/koji/taskinfo?taskID=14326164

rpmlint output:
rpmlint photoqt-1.4-1.fc24.x86_64.rpm 
photoqt.x86_64: W: no-version-in-last-changelog
photoqt.x86_64: W: no-manual-page-for-binary photoqt
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

Please add version to changelog, it's easier to understand what changes happend in which version

photoqt.spec 
photoqt.spec:2: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 2)
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

I'm not sure I like appdata included in SPEC file, not a big deal but it makes SPEC file less clear.

Personally I prefer to use desktop-file-install but guidelines are ok with one of install/validate called, so ok.

{buildroot} and $RPM_BUILD_ROOT are mixed in SPEC file, please choose one for consistency or just remove rm -rf $RPM_BUILD_ROOT from %install section (not needed).

For %build and %install sections I'd prefer to use this snippet (out of source tree build):
%build
mkdir %{_target_platform}
pushd %{_target_platform}
%{cmake} ..
popd

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

%install
make install/fast DESTDIR=%{buildroot} -C %{_target_platform}

Comment 2 Jiri Eischmann 2016-05-31 12:30:27 UTC
(In reply to Jaroslav Reznik from comment #1)
> Name: ok
> Version & release: ok
> Summary: ok
> License: ok (GPLv2+)
> URL: ok
> Sources: ok (SHA256
> 9df3392f0bd746778f119a4da98a32e8ae9ae215564df97d7bcf52efa4d08604)
> 
> BuildRequires: not ok
> 
> BuildRequires:  qt5-qtbase-tds
> BuildRequires:  qt5-qtbase-odbc
> BuildRequires:  qt5-qtbase-ibase
> BuildRequires:  qt5-qtbase-mysql
> BuildRequires:  qt5-qtbase-postgresql
> 
> are not BuildRequires and are not needed to build the package

Ok, removed.
 
> Description: ok
> 
> Files: ok
> Scratch build: builds ok,
> http://koji.fedoraproject.org/koji/taskinfo?taskID=14326164
> 
> rpmlint output:
> rpmlint photoqt-1.4-1.fc24.x86_64.rpm 
> photoqt.x86_64: W: no-version-in-last-changelog
> photoqt.x86_64: W: no-manual-page-for-binary photoqt
> 1 packages and 0 specfiles checked; 0 errors, 2 warnings.
> 
> Please add version to changelog, it's easier to understand what changes
> happend in which version

Added.

> photoqt.spec 
> photoqt.spec:2: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 2)
> 0 packages and 1 specfiles checked; 0 errors, 1 warnings.
> 
> I'm not sure I like appdata included in SPEC file, not a big deal but it
> makes SPEC file less clear.

I wanna stay consistent with my other packages. Once upstream ships the file, I'll remove it.

> Personally I prefer to use desktop-file-install but guidelines are ok with
> one of install/validate called, so ok.
> 
> {buildroot} and $RPM_BUILD_ROOT are mixed in SPEC file, please choose one
> for consistency or just remove rm -rf $RPM_BUILD_ROOT from %install section
> (not needed).

Fixed.

> For %build and %install sections I'd prefer to use this snippet (out of
> source tree build):
> %build
> mkdir %{_target_platform}
> pushd %{_target_platform}
> %{cmake} ..
> popd
> 
> make %{?_smp_mflags} -C %{_target_platform}
> 
> %install
> make install/fast DESTDIR=%{buildroot} -C %{_target_platform}

Fixed.

I've updated the spec file and a new srpm is here: https://dl.dropboxusercontent.com/u/1309518/photoqt-1.4-2.fc24.src.rpm

Comment 3 Jaroslav Reznik 2016-05-31 12:56:53 UTC
Thank you, package is APPROVED.

Comment 4 Gwyn Ciesla 2016-05-31 19:45:28 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/photoqt


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