Spec URL: http://kumarpraveen.fedorapeople.org/sir/sir.spec SRPM URL: http://kumarpraveen.fedorapeople.org/sir/sir-2.1.1-1.fc14.src.rpm Description: SIR (Simple Image Resizer) is a simple application for resizing images,inspired by GTPY - ImageResizer But it use C++/QT and QImage class to convert the images.it also can rotate your images.
Use macros in source0 INSTALL shouldn't be packaged License tag must be GPLv2+. Don't just read the license. Look at the source file headers. rpmlint warnings to be fixed: sir.spec:21: W: macro-in-comment %{name} (rpmlint on spec file) sir.x86_64: W: unstripped-binary-or-object /usr/bin/sir sir.x86_64: E: zero-length /usr/share/doc/sir-2.1.1/TODO sir.x86_64: W: no-manual-page-for-binary sir "#remove binary rm %{name}" Why is this necessary? "%{_datadir}/applications/*.desktop" better not to use a catch all for a single desktop file.
(In reply to comment #1) > Use macros in source0 > > INSTALL shouldn't be packaged > > License tag must be GPLv2+. Don't just read the license. Look at the source > file headers. Fixed > > rpmlint warnings to be fixed: > > sir.spec:21: W: macro-in-comment %{name} (rpmlint on spec file) > > sir.x86_64: W: unstripped-binary-or-object /usr/bin/sir Can't understand what's unstripped-binary-or-object mean, please explain. > sir.x86_64: E: zero-length /usr/share/doc/sir-2.1.1/TODO TODO is removed from doc section > sir.x86_64: W: no-manual-page-for-binary sir > > "#remove binary > rm %{name}" > > Why is this necessary? Source already contain a binary, before build section we have to remove all binary IIRC. > > "%{_datadir}/applications/*.desktop" > > better not to use a catch all for a single desktop file. Fixed Here is updated spec and srpm urls Spec URL: http://kumarpraveen.fedorapeople.org/sir/sir.spec SRPM URL: http://kumarpraveen.fedorapeople.org/sir/sir-2.1.1-2.fc14.src.rpm rpmlint Output : $ rpmlint sir.spec sir.spec: W: invalid-url Source0: http://sir.googlecode.com/files/sir_2.1.1.tar.gz HTTP Error 404: Not Found 0 packages and 1 specfiles checked; 0 errors, 1 warnings. (may be due to proxy its not able to resolve url)
"Can't understand what's unstripped-binary-or-object mean, please explain." Debuginfo package is not being generated. Confirm with mock and check your Makefile/configure scripts to see why. "Source already contain a binary, before build section we have to remove all binary IIRC." Inform upstream of this. Merely not including it %files would omit it out of the package. Also rpmlint should be run on spec file, srpm and binary rpm
(In reply to comment #3) > Debuginfo package is not being generated. Confirm with mock and check your > Makefile/configure scripts to see why. In my system debuginfo package is generated. Please take a look http://kumarpraveen.fedorapeople.org/sir/sir-debuginfo-2.1.1-2.fc14.i686.rpm > Inform upstream of this. Merely not including it %files would omit it out of > the package. I informed to upstream for removing the binaries. > Also rpmlint should be run on spec file, srpm and binary rpm $rpmlint sir.spec ../SRPMS/sir-2.1.1-2.fc14.src.rpm ../RPMS/i686/sir-2.1.1-2.fc14.i686.rpm sir.spec: W: invalid-url Source0: http://sir.googlecode.com/files/sir_2.1.1.tar.gz HTTP Error 404: Not Found sir.src: W: invalid-url Source0: http://sir.googlecode.com/files/sir_2.1.1.tar.gz HTTP Error 404: Not Found sir.i686: W: no-manual-page-for-binary sir 2 packages and 1 specfiles checked; 0 errors, 3 warnings.
Done a scratch build http://koji.fedoraproject.org/koji/taskinfo?taskID=2924176 No debuginfo issue. So ignore that. Minor nit pick "But used C++/QT" Should be " but uses" in the description. I will let Arun SAG do the full review and approve since he has taken it.
Praveen, Why wouldn't you use 'make install' instead of manually copying binaries and data to their corresponding directories? Also you seems to be ignoring the following files, which actually installs when using the default "make install" images/sir-128x128.png images/sir-32x32.png images/sir-64x64.png images/sir.ico sir_service.desktop
Thanks rahul, for the informal review :-). I will do the full review.
(In reply to comment #6) > Praveen, > > Why wouldn't you use 'make install' instead of manually copying binaries and > data to their corresponding directories? > > Also you seems to be ignoring the following files, which actually installs when > using the default "make install" > > images/sir-128x128.png > images/sir-32x32.png > images/sir-64x64.png > images/sir.ico > sir_service.desktop This MakeFile is generated using qmake and when I am using "make install DESTDIR=/path/to/install" then it is not creating directories(bin,pixmaps,share ..etc) on given path, but a GNU Makefile do. If above files are important then i can add manually.
(In reply to comment #8) > > This MakeFile is generated using qmake and when I am using "make install > DESTDIR=/path/to/install" then it is not creating directories(bin,pixmaps,share > ..etc) on given path, but a GNU Makefile do. If above files are important then > i can add manually. A quick look at the makefile shows that, it is using INSTALL_ROOT as prefix. Remove everything in install section and try using the following lines "make install INSTALL_ROOT=$RPM_BUILD_ROOT desktop-file-install --dir=$RPM_BUILD_ROOT%{_datadir}/applications %{name}.desktop I do not know whether these files are important for the application, the application is working fine without them; But it desirable to include them as the original make file is installing them. sir_service.desktop seems to adding this application to "Open with" dialog box.
Forgot! Add the following too, desktop-file-install desktop-file-install --dir=$RPM_BUILD_ROOT {_datadir}/applications %{name}_service.desktop
Thanks for suggestion, Fixed all Issues, here is updated spec and srpm SPEC URL: http://kumarpraveen.fedorapeople.org/sir/sir.spec SRPM URL: http://kumarpraveen.fedorapeople.org/sir/sir-2.1.1-3.fc14.src.rpm rpmlint output: $ rpmlint sir.spec ../SRPMS/sir-2.1.1-3.fc14.src.rpm ../RPMS/i686/sir-2.1.1-3.fc14.i686.rpm sir.spec: W: invalid-url Source0: http://sir.googlecode.com/files/sir_2.1.1.tar.gz HTTP Error 404: Not Found sir.src: W: invalid-url Source0: http://sir.googlecode.com/files/sir_2.1.1.tar.gz HTTP Error 404: Not Found sir.i686: W: no-manual-page-for-binary sir 2 packages and 1 specfiles checked; 0 errors, 3 warnings.
[+] OK [X] NOT OKAY [?] Investigate the issue before committing the package [-] NA [+] Package meets naming and packaging guidelines [+] Spec file matches base package name. [+] Spec has consistent macro usage. [+] Meets Packaging Guidelines. [+] License [+] License field in spec matches [+] License file included in package [+] Spec in American English [+] Spec is legible. [+] Sources match upstream [zer0c00l@gnubox SPECS]$ md5sum ~/Downloads/sir_2.1.1.tar.gz bbb0526a8b828379449468066d166e04 /home/zer0c00l/Downloads/sir_2.1.1.tar.gz [zer0c00l@gnubox SPECS]$ -- done [-] Package needs ExcludeArch [+] BuildRequires correct [+] Spec handles locales/find_lang [-] Package is relocatable and has a reason to be. [+] Package has %defattr and permissions on files is good. [-] Package has a correct %clean section. [-] Package has correct buildroot [+] Package is code or permissible content. [-] Doc subpackage needed/used. [+] Packages %doc files don't affect runtime. [-] Headers/static libs in -devel subpackage. [-] Spec has needed ldconfig in post and postun [-] .pc files in -devel subpackage/requires pkgconfig [-] .so files in -devel subpackage. [-] -devel package Requires: %{name} = %{version}-%{release} [-] .la files are removed. [+] Package is a GUI app and has a .desktop file [+] Package compiles and builds on at least one arch. http://koji.fedoraproject.org/koji/taskinfo?taskID=2979037 [+] Package has no duplicate files in %files. [+] Package doesn't own any directories other packages own. [+] Package owns all the directories it creates. [?] rpmlint output [zer0c00l@gnubox sir]$ rpmlint ~/rpmbuild/RPMS/i686/sir-2.1.1-3.fc14.i686.rpm sir.i686: W: no-manual-page-for-binary sir 1 packages and 0 specfiles checked; 0 errors, 1 warnings. [zer0c00l@gnubox sir]$ rpmlint ~/rpmbuild/RPMS/i686/sir-debuginfo-2.1.1-3.fc14.i686.rpm sir-debuginfo.i686: W: hidden-file-or-dir /usr/src/debug/sir/.moc sir-debuginfo.i686: W: hidden-file-or-dir /usr/src/debug/sir/.moc sir-debuginfo.i686: W: hidden-file-or-dir /usr/src/debug/sir/.ui sir-debuginfo.i686: W: hidden-file-or-dir /usr/src/debug/sir/.ui 1 packages and 0 specfiles checked; 0 errors, 4 warnings. Check the hidden-file-or-dir warning, ask the upstream about the purpose of their existence before committing the package. SHOULD Items: [+] Should build in mock. [+] Should build on all supported archs [+] Should function as described. [-] Should have sane scriptlets. [-] Should have subpackages require base package with fully versioned depend. [+] Should have dist tag [+] Should package latest version [-] check for outstanding bugs on package. (For core merge reviews) XXX APPROVED XXX
New Package SCM Request ======================= Package Name: sir Short Description: A simple application for resizing images Owners: kumarpraveen Branches: f13 f14 f15
Git done (by process-git-requests).
sir-2.1.1-3.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/sir-2.1.1-3.fc15
sir-2.1.1-3.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/sir-2.1.1-3.fc14
sir-2.1.1-3.fc13 has been submitted as an update for Fedora 13. https://admin.fedoraproject.org/updates/sir-2.1.1-3.fc13
sir-2.1.1-3.fc14 has been pushed to the Fedora 14 testing repository.
sir-2.1.1-3.fc15 has been pushed to the Fedora 15 stable repository.
sir-2.1.1-3.fc13 has been pushed to the Fedora 13 stable repository.
sir-2.1.1-3.fc14 has been pushed to the Fedora 14 stable repository.
sir-2.4-1.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/sir-2.4-1.fc17