Bug 246460 - Review request: qtpfsgui - A Qt4 graphical user interface that provides a workflow for HDR imaging
Review request: qtpfsgui - A Qt4 graphical user interface that provides a wor...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
http://www.silfreed.net/download/repo...
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-07-02 10:00 EDT by Douglas E. Warner
Modified: 2007-11-30 17:12 EST (History)
3 users (show)

See Also:
Fixed In Version: 1.8.9-5.fc7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-07-16 12:59:34 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Douglas E. Warner 2007-07-02 10:00:42 EDT
Spec URL: 
http://www.silfreed.net/download/repo/packages/qtpfsgui/qtpfsgui.spec
SRPM URL: 
http://www.silfreed.net/download/repo/packages/qtpfsgui/qtpfsgui-1.8.9-2.src.rpm


%description
Qtpfsgui is a Qt4 graphical user interface that provides a workflow for
HDR imaging.


$ rpmlint RPMS/qtpfsgui-*1.8.9-2*.rpm SRPMS/qtpfsgui-1.8.9-2.src.rpm
$ echo $?
0
Comment 1 Douglas E. Warner 2007-07-02 10:01:30 EDT
Currently working on bug 241403 as my primary sponsorship bug.
Comment 2 Douglas E. Warner 2007-07-02 14:05:14 EDT
Being sponsored by mtasaka@ioa.s.u-tokyo.ac.jp (bug 241403)
Comment 3 Mamoru TASAKA 2007-07-02 14:09:47 EDT
Very rapid comment:
* Don't write the packages to Requires explicitly which are selected
  by the dependency check of the libraries checked by rpmbuild itself.
  Perhaps all the Requires currently written is not needed.

( Currently I may have no time to review this)
Comment 4 Douglas E. Warner 2007-07-02 14:47:59 EDT
(In reply to comment #3)
> Very rapid comment:
> * Don't write the packages to Requires explicitly which are selected
>   by the dependency check of the libraries checked by rpmbuild itself.
>   Perhaps all the Requires currently written is not needed.

Sorry about that; fixed up now.

> ( Currently I may have no time to review this)

No problems.  Thanks for the sponsorship.


Spec URL: 
http://www.silfreed.net/download/repo/packages/qtpfsgui/qtpfsgui.spec
SRPM URL: 
http://www.silfreed.net/download/repo/packages/qtpfsgui/qtpfsgui-1.8.9-3.src.rpm

%changelog
* Mon Jul 02 2007 Douglas E. Warner <silfreed@silfreed.net> 1.8.9-3
- removing explicit Requires
Comment 5 Bruno Postle 2007-07-03 19:15:06 EDT
Looks good to me, I was going to submit this myself.  My suggestions, though you
don't have to follow them:

The summary could be a bit less cryptic:

  A graphical tool for creating and tone-mapping HDR images

..and the description should say a bit more:

  Qtpfsgui is a graphical program for assembling bracketed photos into High
  Dynamic Range (HDR) images.  It also provides a number of tone-mapping
  operators for creating low dynamic range versions of HDR images.

I would fix the newlines in %prep and not %install, but this isn't important.

You can call 'desktop-file-install' with '--delete-original', then you don't
have to manually delete it afterwards
Comment 6 Douglas E. Warner 2007-07-04 09:04:02 EDT
(In reply to comment #5)
> The summary could be a bit less cryptic:
> 
>   A graphical tool for creating and tone-mapping HDR images
> 
> ..and the description should say a bit more:
> 
>   Qtpfsgui is a graphical program for assembling bracketed photos into High
>   Dynamic Range (HDR) images.  It also provides a number of tone-mapping
>   operators for creating low dynamic range versions of HDR images.

Thanks for the improved summary and description; I was having a hard time 
coming up with decent ones myself (as you saw ;).

> I would fix the newlines in %prep and not %install, but this isn't 
important.

Okay.  Is there any general guideline as to what should be done in %prep and 
what should be done in %install?  I've mostly kept my %prep sections to 
run %setup and %patch, but since I was just fixing a file that was being 
pulled in by %doc, I could see how it would make more sense to be moved there 
since it wasn't being installed by %install.

> You can call 'desktop-file-install' with '--delete-original', then you don't
> have to manually delete it afterwards

Excellent!  I'll pull that in as well.

I should have new packages up shortly.
Comment 7 Douglas E. Warner 2007-07-04 09:15:27 EDT
Currently uploading the spec file.

Spec URL: 
http://www.silfreed.net/download/repo/packages/qtpfsgui/qtpfsgui.spec
SRPM URL: 
http://www.silfreed.net/download/repo/packages/qtpfsgui/qtpfsgui-1.8.9-4.src.rpm

%changelog
* Wed Jul 04 2007 Douglas E. Warner <silfreed@silfreed.net> 1.8.9-4
- cleaning up summary and description
- fixing newlines in prep instead of install
- updating desktop-file-install to call --delete-original to remove that
  manual step
Comment 8 Bruno Postle 2007-07-04 14:32:10 EDT
(In reply to comment #6)
> Okay.  Is there any general guideline as to what should be done in %prep and 
> what should be done in %install?

Not anywhere I know.  The packaging guidelines do suggest fixing newline
characters in %prep, though I'm sure it doesn't make any practical difference
where you do it:

http://fedoraproject.org/wiki/Packaging/Guidelines#head-41d4336fa1af8d74500eb89d3a22410cccc4117d

Comment 9 Mamoru TASAKA 2007-07-07 07:33:50 EDT
Assigning.
Comment 10 Mamoru TASAKA 2007-07-07 08:16:38 EDT
For 1.8.9-4.fc8: almost okay.

* Timestamps
  - Keep timestamps on files under %_datadir/%name/html.
    For this package, the following should work
--------------------------------------------------------
make install -e INSTALL_ROOT=%{buildroot} COPY="cp -pf"
--------------------------------------------------------
    (Check the file "Makefile" on top builddir).

* GTK icon cache
  - Needs updating (please check: "GTK+ icon cache" of
    http://fedoraproject.org/wiki/Packaging/ScriptletSnippets )

* Documents
  - "INSTALL" is for people who want to rebuild this package
    by themselves andd is not needed for rpm users.
Comment 11 Douglas E. Warner 2007-07-10 12:18:25 EDT
Spec URL: 
http://www.silfreed.net/download/repo/packages/qtpfsgui/qtpfsgui.spec
SRPM URL: 
http://www.silfreed.net/download/repo/packages/qtpfsgui/qtpfsgui-1.8.9-5.src.rpm

%changelog
* Tue Jul 10 2007 Douglas E. Warner <silfreed@silfreed.net> 1.8.9-5
- preserving timestamps on install
- removed INSTALL file from docs
- updating GTK icon cache
Comment 12 Mamoru TASAKA 2007-07-10 12:54:53 EDT
Okay.

-----------------------------------------------
  This package (qtpfsgui) is APPROVED by me
-----------------------------------------------
Comment 13 Douglas E. Warner 2007-07-10 13:02:34 EDT
New Package CVS Request
=======================
Package Name: qtpfsgui
Short Description: A graphical tool for creating and tone-mapping HDR images
Owners: silfreed@silfreed.net
Branches: FC-6 F-7
Comment 14 Kevin Fenzi 2007-07-10 18:44:18 EDT
cvs done.
Comment 15 Fedora Update System 2007-07-11 11:18:41 EDT
qtpfsgui-1.8.9-5.fc7 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.
Comment 16 Fedora Update System 2007-07-16 12:59:29 EDT
qtpfsgui-1.8.9-5.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.

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