https://bugzilla.redhat.com/show_bug.cgi?id=1024993
Bug ID: 1024993 Summary: Review Request: lin_guider - Astronomical autoguiding program Product: Fedora Version: rawhide Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: lukashjames@gmail.com QA Contact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org
Spec URL: http://krypt3r.net63.net/packages/fedora/lin_guider.spec SRPM URL: http://krypt3r.net63.net/packages/fedora/lin_guider-2.9.0-1.fc17.src.rpm Description: Lin-guider is an astronomical autoguiding program for Linux. It supports Philips, Logitech, uvc webcams, QHY5, QHY5-II-M, QHY5-II, QHY6, DSI2PRO astrocams for video and FTDI chip-based, parallel port-based (LPT) and QHY5, QHY6 astrocam devices for pulse guiding. Fedora Account System Username: krypt3r Blocks: FE-NEEDSPONSOR
It is my first package, so I need a sponsor. Depends on the package 'firmware-ccd' (will be soon).
https://bugzilla.redhat.com/show_bug.cgi?id=1024993
Lukash James lukashjames@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841 (FE-NEEDSPONSOR)
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=1024993
--- Comment #1 from Lukash James lukashjames@gmail.com --- SRPM URL changed to http://krypt3r.net63.net/packages/fedora/lin_guider-2.9.0-1.fc17.src.rpm.bz2
https://bugzilla.redhat.com/show_bug.cgi?id=1024993
--- Comment #2 from Antonio Trande anto.trande@gmail.com --- Hi James.
(In reply to Lukash James from comment #1)
SRPM URL changed to http://krypt3r.net63.net/packages/fedora/lin_guider-2.9.0-1.fc17.src.rpm.bz2
- Why ?
For an handy review, it would be better to have two direct links, one for the .spec file and one for the .srpm .
- Why a manual installation ? Does '%make DESTDIR="" install' not work ?
https://bugzilla.redhat.com/show_bug.cgi?id=1024993
--- Comment #3 from Lukash James lukashjames@gmail.com --- (In reply to Antonio Trande from comment #2)
Hi James.
(In reply to Lukash James from comment #1)
SRPM URL changed to http://krypt3r.net63.net/packages/fedora/lin_guider-2.9.0-1.fc17.src.rpm.bz2
- Why ?
Hello. URL changed because I can't get RPM file from my hosting (only bzipped). May be I can edit the description?
For an handy review, it would be better to have two direct links, one for the .spec file and one for the .srpm .
- Why a manual installation ?
Does '%make DESTDIR="" install' not work ?
After building lin_guider binary started from build directory. Makefile haven't 'install' target.
PS. I am not an upstream developer.
https://bugzilla.redhat.com/show_bug.cgi?id=1024993
--- Comment #4 from Lukash James lukashjames@gmail.com --- (In reply to Lukash James from comment #3)
URL changed because I can't get RPM file from my hosting (only bzipped). May be I can edit the description?
Bug fixed with .htaccess. SRPM URL in 'Description' now valid.
https://bugzilla.redhat.com/show_bug.cgi?id=1024993
--- Comment #5 from Antonio Trande anto.trande@gmail.com --- (In reply to Lukash James from comment #3)
- Why a manual installation ?
Does '%make DESTDIR="" install' not work ?
After building lin_guider binary started from build directory. Makefile haven't 'install' target.
PS. I am not an upstream developer.
Consider to leave a comment in the .spec file to describe the reasons of your choices, above all if they fall outside of packaging guidelines.
- You don't need of '%defattr(-, root, root)' line http://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions
- Please remove the lines
#ln -s /opt/gm_software/lin_guider/lg-wrapper.sh $RPM_BUILD_ROOT%{_bindir}/lin_guider #ln -s /opt/gm_software/lin_guider/man/man1/lin_guider.1.gz $RPM_BUILD_ROOT%{_mandir}/man1/lin_guider.1.gz #ln -s /opt/gm_software/lin_guider/man/ru/man1/lin_guider.1.gz $RPM_BUILD_ROOT%{_mandir}/ru/man1/lin_guider.1.gz #install contrib/{mc.sh,mc.csh} $RPM_BUILD_ROOT%{_sysconfdir}/profile.d
if they are not necessary.
- Your package provides a .desktop file; validate it in a %check section.
http://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usag...
- Consider that starting from f20, every package docdir subdirectory will be unversioned. See https://fedoraproject.org/wiki/Changes/UnversionedDocdirs
https://bugzilla.redhat.com/show_bug.cgi?id=1024993
--- Comment #6 from Lukash James lukashjames@gmail.com --- (In reply to Antonio Trande from comment #5)
Consider to leave a comment in the .spec file to describe the reasons of your choices, above all if they fall outside of packaging guidelines.
- You don't need of '%defattr(-, root, root)' line
- Please remove the lines
- Your package provides a .desktop file; validate it in a %check section.
- Consider that starting from f20, every package docdir subdirectory will be
unversioned.
Thank you for your comments, Antonio. I fixed all of them (I think) and updated files in 'Description' section. I have a question. Should I add new entries in %changelog if I fixed anything?
https://bugzilla.redhat.com/show_bug.cgi?id=1024993
--- Comment #7 from Antonio Trande anto.trande@gmail.com --- (In reply to Lukash James from comment #6)
I have a question. Should I add new entries in %changelog if I fixed anything?
- You may update and replace the existing data line since your package is not built yet. See http://fedoraproject.org/wiki/Packaging:Guidelines#Multiple_Changelog_Entrie...
- Move the %check section after %install Was 'desktop-file-install' line already in your first .spec file ? If yes, desktop-file-validate is useless at this point. :P
'desktop-file-install' already validates the .desktop files automatically.
- Move the line
%{!?_pkgdocdir: %global _pkgdocdir %{_docdir}/%{name}-%{version}}
at the top.
https://bugzilla.redhat.com/show_bug.cgi?id=1024993
--- Comment #8 from Lukash James lukashjames@gmail.com --- (In reply to Antonio Trande from comment #7)
Was 'desktop-file-install' line already in your first .spec file ? If yes, desktop-file-validate is useless at this point. :P
No, 'install ...' was there :)
'desktop-file-install' already validates the .desktop files automatically.
Oh, I see. I shall fix it.
- Move the line
%{!?_pkgdocdir: %global _pkgdocdir %{_docdir}/%{name}-%{version}}
at the top.
Before 'Name' and 'Version'? But will it work? 'Name' and 'Version' not defined in the first line.
https://bugzilla.redhat.com/show_bug.cgi?id=1024993
--- Comment #9 from Antonio Trande anto.trande@gmail.com --- (In reply to Lukash James from comment #8)
(In reply to Antonio Trande from comment #7)
Was 'desktop-file-install' line already in your first .spec file ? If yes, desktop-file-validate is useless at this point. :P
No, 'install ...' was there :)
So I'm sorry.
- Move the line
%{!?_pkgdocdir: %global _pkgdocdir %{_docdir}/%{name}-%{version}}
at the top.
Before 'Name' and 'Version'? But will it work? 'Name' and 'Version' not defined in the first line.
Good question. Yes, it works; I've used it myself with another one package. I think because 'rpm' itself defines numerous macros. If you add "%dump" to the beginning of your spec file, process with rpm, you can examine them.
https://bugzilla.redhat.com/show_bug.cgi?id=1024993
--- Comment #10 from Lukash James lukashjames@gmail.com --- (In reply to Antonio Trande from comment #9)
Good question. Yes, it works; I've used it myself with another one package. I think because 'rpm' itself defines numerous macros. If you add "%dump" to the beginning of your spec file, process with rpm, you can examine them.
Thanks. Spec & SRPM updated.
https://bugzilla.redhat.com/show_bug.cgi?id=1024993
--- Comment #11 from Antonio Trande anto.trande@gmail.com ---
It is my first package, so I need a sponsor. Depends on the package 'firmware-ccd' (will be soon).
I'm not a sponsor so I can't do an official review of your first packages (you need to be sponsored first of all). However, I can to do an initial review to relieve your (prospective) sponsor of some of his work, as soon as even the 'firmware-ccd' package will be ready.
For everything else it depends by yourself and by your sponsor (see http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group). :)
package-review@lists.fedoraproject.org