Bug 428959 - Review Request: coriander - Control a 1394 digital camera interactively
Summary: Review Request: coriander - Control a 1394 digital camera interactively
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Hans de Goede
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 239043
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-01-16 14:42 UTC by Tim Niemueller
Modified: 2008-01-28 16:01 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-01-21 08:46:58 UTC
Type: ---
Embargoed:
hdegoede: fedora-review+
dennis: fedora-cvs+


Attachments (Terms of Use)

Description Tim Niemueller 2008-01-16 14:42:52 UTC
Spec URL: http://fedorapeople.org/~timn/coriander/coriander.spec
SRPM URL: http://fedorapeople.org/~timn/coriander/coriander-2.0.0-rc5.4.cvs20080116.fc8.src.rpm
Description: Coriander is a GUI that let you control your 1394 digital video camera interactively. It features SDL display, FTP image posting, file saving, and Real streaming. It is for IIDC cameras, not for consumer grade DV cameras.

Comment 1 Hans de Goede 2008-01-18 14:11:45 UTC
Full review done, results:

Must Fix
--------
- Remove the following lines from src/build_menus.c:
  // add edge sense option
  glade_menuitem = gtk_menu_item_new_with_label (_("Edge Sense"));
  gtk_widget_show (glade_menuitem);
  gtk_menu_append (GTK_MENU (new_menu), glade_menuitem);
  g_signal_connect ((gpointer) glade_menuitem, "activate",
                      G_CALLBACK (on_bayer_menu_activate),
                      (int*)DC1394_BAYER_METHOD_EDGESENSE);

  As libdc1394-2.0.1 no longer supports this

- Change Release to "0.4.rc5%{?cvs_snapshot}%{?dist}", as per the versioning 
  guidelines so that upgrading to 2.0.0 final can happen without epoch.

- Drop the unnecessary explicit: "Requires: libdc1394 >= 2.0.0" 2.0.0 has a 
  different soname then libdc1394 v1, so there is no need for this.

- Reword "and Real streaming" in description, this made me think of Real
  video (realmedia, .rm files) streaming which is patented.

- Don't use an url for cvs snapshots, as those cannot be downloaded from such an
  URL

- Drop buildrequires ffmpeg-devel, we don't have ffmpeg-devel in Fedora

- Change "--vendor rpmforge" desktop-file-install argument to "--vendor fedora"
  and drop the obsolete "--add-category X-Red-Hat-Base" argument

- Add (as comment above Source0) instructions on howto recreate the tarbal from 
  CVS (so cvs co command, any commands used to clean the checkout, etc.)


Should Fix
----------

- Add an Icon= line to the .desktop fil, install a suitable icon under
  /usr/share/icons/hicolor/48x48/apps, and add icon cache update scriptlets  


Not checked
-----------

- sources matching due to missing reproduction instructions for the tarbal.


Comment 2 Tim Niemueller 2008-01-19 00:27:41 UTC
First, thanks for the thorough review!

- Using the just-released version 2.0.0-rc6. No CVS snapshot no more
- EdgeSense auto-detection has been added in this version which eliminates the
first remark
- Changed the release accordingly to the mentioned version number
- Dropped explicit require for libdc1394 >= 2.0.0
- Reworded to "video streaming"
- URL is now ok since we are using the release candidate tarball
- ffmpeg-devel no longer required
- desktop-file-install parameters fixed
- Added Icon line to desktop file
- Added ftplib to BR to build coriander with FTP support

The new package and spec file can be found at
http://fedorapeople.org/~timn/coriander/.

Comment 3 Hans de Goede 2008-01-19 18:40:05 UTC
All must fix items fixed: Approved.

I still have one should fix left though, please consider fixing this before import:

* The preferred Source URL for sourceforge downloads (see guidelines) says that 
  the Source tag should be:
Source: http://downloads.sourceforge.net/coriander/coriander-%{real_version}.tar.gz

Notice that I tried the currently used URL with "spectool -g coriander.spec",
and it failed, whereas the prefered form as above works.


Comment 4 Tim Niemueller 2008-01-19 20:23:32 UTC
I have changed the spec file accordingly and uploaded the new files.

Comment 5 Tim Niemueller 2008-01-19 20:24:02 UTC
New Package CVS Request
=======================
Package Name: coriander
Short Description: Control a 1394 digital camera interactively
Owners: timn
Branches: F-8
InitialCC: timn
Cvsextras Commits: yes

Comment 6 Kevin Fenzi 2008-01-19 20:54:22 UTC
cvs done.

Comment 7 Tim Niemueller 2008-01-20 00:02:12 UTC
I have imported the sources. I tried a build not realizing that it cannot find
libdc1394-devel until it's pushed... Going to resubmit the build as soon as the
libdc1394 package has landed.

Comment 8 Hans de Goede 2008-01-20 08:56:26 UTC
Note that for F-8 you can send a mail to rel-eng <the-symbol-knows-as-at>
fedoraproject.org asking the package to be included into the F-8 buildroot, even
though it hasn't hit updates yet. This is usually much faster then waiting for
it to be pushed as an update.

The proper wording is tagging into the buildroot and always include full name
and EVR, so a possible subject for such a mail would be:
"Please tag libdc1394-2.0.1-3.fc8 for F-8 buildroot inclusion"

And then some explanation why (to build coriander) ihn the body.


Comment 9 Tim Niemueller 2008-01-21 08:46:58 UTC
Builds done. Update push is requested at
https://admin.fedoraproject.org/updates/F8/pending/coriander-2.0.0-0.6.rc6.fc8

Closing this as NEXTRELEASE. Thanks for the review Hans.

Comment 10 Tim Niemueller 2008-01-28 14:39:51 UTC
Package Change Request
======================
Package Name: coriander
New Branches: F-7 EL-5

Comment 11 Dennis Gilmore 2008-01-28 16:01:49 UTC
CVS Done


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