[Bug 1024993] New: Review Request: lin_guider - Astronomical autoguiding program

[Red Hat Bugzilla]

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: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail],
                    [e-mail]

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).

[Bug 1024993] Review Request: lin_guider - Astronomical autoguiding program

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1024993

Lukash James  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

[Bug 1024993] Review Request: lin_guider - Astronomical autoguiding program

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1024993

--- Comment #1 from Lukash James  ---
SRPM URL changed to
http://krypt3r.net63.net/packages/fedora/lin_guider-2.9.0-1.fc17.src.rpm.bz2

[Bug 1024993] Review Request: lin_guider - Astronomical autoguiding program

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1024993

--- Comment #2 from Antonio Trande  ---
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 ?

[Bug 1024993] Review Request: lin_guider - Astronomical autoguiding program

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1024993

--- Comment #3 from Lukash James  ---
(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.

[Bug 1024993] Review Request: lin_guider - Astronomical autoguiding program

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1024993

--- Comment #4 from Lukash James  ---
(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.

[Bug 1024993] Review Request: lin_guider - Astronomical autoguiding program

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1024993

--- Comment #5 from Antonio Trande  ---
(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

[Bug 1024993] Review Request: lin_guider - Astronomical autoguiding program

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1024993

--- Comment #6 from Lukash James  ---
(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?

[Bug 1024993] Review Request: lin_guider - Astronomical autoguiding program

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1024993

--- Comment #7 from Antonio Trande  ---
(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.

[Bug 1024993] Review Request: lin_guider - Astronomical autoguiding program

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1024993

--- Comment #8 from Lukash James  ---
(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.

[Bug 1024993] Review Request: lin_guider - Astronomical autoguiding program

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1024993

--- Comment #9 from Antonio Trande  ---
(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.

[Bug 1024993] Review Request: lin_guider - Astronomical autoguiding program

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1024993

--- Comment #10 from Lukash James  ---
(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.

[Bug 1024993] Review Request: lin_guider - Astronomical autoguiding program

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1024993

--- Comment #11 from Antonio Trande  ---
> 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). :)