Bug 485636 - Review Request: cutecom - A GUI application for serial port communications
Summary: Review Request: cutecom - A GUI application for serial port communications
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-02-15 18:23 UTC by Jose Luis
Modified: 2009-02-24 20:44 UTC (History)
3 users (show)

Fixed In Version: 0.20.0-3.fc10
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-02-24 20:44:59 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Jose Luis 2009-02-15 18:23:08 UTC
SPEC URL: http://babel.isa.uma.es/mrpt/src-repo/rpm/cutecom.spec
SRPM URL: http://babel.isa.uma.es/mrpt/src-repo/rpm/cutecom-0.20.0-1.fc10.src.rpm
Description:
CuteCom is a graphical serial terminal, like minicom or Hyperterminal on 
Windows. 
It is aimed mainly at hardware developers or other people who need 
a terminal to talk to their devices. 

It is free software and distributed under 
the GNU General Public License Version 2, which can find in the file COPYING. 
It is written using the Qt library by Trolltech.

Comment 1 manuel wolfshant 2009-02-16 19:23:30 UTC
What is the reason of using
 # Upstream script does not install the .desktop file if KDE is not installed, 
 # so we install it manually:
 install -D -m 644 $(pwd)/cutecom.desktop ${RPM_BUILD_ROOT}%{_datadir}/applications/cutecom.desktop 
 # Validate .desktop files:
 desktop-file-validate ${RPM_BUILD_ROOT}%{_datadir}/applications/cutecom.desktop

instead of desktop-file-install which install and validates in a sigle step ?


According to the source files, the license should be GPLv2+. All of them include:
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
    the Free Software Foundation; either version 2 of the License, or
    (at your option) any later version.

You must include the license (COPYING) as %doc. I would also include the changelog and the "todo" files but I wouldn't cry if you choose to ignore them.

Comment 2 Jose Luis 2009-02-17 08:55:22 UTC
You're right Manuel. I've changed it to desktop-file-install. 
I've also fixed the other issues.

This is the new 0.20.0-2 revision:

SPEC URL: http://babel.isa.uma.es/mrpt/src-repo/rpm/cutecom.spec
SRPM URL: http://babel.isa.uma.es/mrpt/src-repo/rpm/cutecom-0.20.0-2.fc10.src.rpm

Comment 3 Mamoru TASAKA 2009-02-19 17:47:09 UTC
Ah... you can't assign what you submitted to yourself.

Comment 4 Mamoru TASAKA 2009-02-19 18:10:35 UTC
Assigning to myself.

Some notes.

* SourceURL
  - I recomment to use %{version} tag in Source because with
    this you probably won't have to modify SourceURL when
    the version is upgraded, ref:
    https://fedoraproject.org/wiki/Packaging/SourceURL#Using_.25.7Bversion.7D

* Description
-----------------------------------------------------------
It is free software and distributed under 
the GNU General Public License Version 2, which can find in the file COPYING. 
-----------------------------------------------------------
  - is not needed because we can check this by "License" tag in
    the rebuilt rpm.
  - I also think that
-----------------------------------------------------------
It is written using the Qt library by Trolltech.
-----------------------------------------------------------
    is not needed.

* Desktop file issue
  - From build.log
-----------------------------------------------------------
+ desktop-file-install --dir /builddir/build/BUILDROOT/cutecom-0.20.0-2.fc11.i386/usr/share/applications/ /builddir/build/BUILD/cutecom-0.20.0/cutecom.desktop
/builddir/build/BUILDROOT/cutecom-0.20.0-2.fc11.i386/usr/share/applications/cutecom.desktop: warning: value "" for key "Path" in group "Desktop Entry" does not look like an absolute path
-----------------------------------------------------------
    I guess Path= item in cutecom.desktop is not needed.

  - By the way, cutecom.desktop does not have any Categories.
    Please add the proper one.

  ? cutecom.desktop specifies "openterm" as Icon, however
    gnome-icon-theme 2.25.91 does not have openterm.{png,svg} (2.24.x
    had this icon). 

    Maybe with formal 2.26 gnome-icon-theme release
    openterm.{png,svg} will reintroduced again, however
    it may be better that you change Icon item to "utilities-terminal"
    (actually in gnome-icon-theme 2.24.x, openterm.{png,svg} was
    symlinks to utilities-terminal.{png,svg}.

  ? Also please check is the empty line "MimeType=" in
    cutecom.desktop is needed. If this "MimeType=" line can be
    removed, then calling "update-desktop-database" on scriptlets
    is no longer needed, ref:
    https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#desktop-database

Comment 5 Jose Luis 2009-02-19 22:59:04 UTC
Thanks Mamoru!

> Ah... you can't assign what you submitted to yourself.
Okay... wasn't sure about that!

> SourceURL
Changes to %{name}-%{version}

> Description
Shortened.

> Desktop file.
This file has many issues... I've finally left it as:

desktop-file-install --remove-key=Path --remove-key=Encoding --remove-key=BinaryPattern --remove-key=TerminalOptions --add-category=System --dir ${RPM_BUILD_ROOT}%{_datadir}/applications/ $(pwd)/cutecom.desktop

which fixes almost everything. The only remaining detail is the icon, but I'm not sure on whether it can be changed via desktop-file-install, or I should modify it "manually" (via 'sed', etc...) from the script.


The new revision is here:

SPEC: http://babel.isa.uma.es/mrpt/src-repo/rpm/cutecom.spec
SRPM: http://babel.isa.uma.es/mrpt/src-repo/rpm/cutecom-0.20.0-3.fc10.src.rpm

Comment 6 Mamoru TASAKA 2009-02-20 16:42:38 UTC
Looks good. Some comments.

- Would you split a long command line into several lines like below
  so that we can read it easier line below?
----------------------------------------------------------------
desktop-file-install \
   --remove-key=Path --remove-key=Encoding \
   --removekey=BinaryPattern --remove-key=TerminalOptions \
   --add-category=System \
   --dir ${RPM_BUILD_ROOT}%{_datadir}/applications/ \
   $(pwd)/cutecom.desktop
----------------------------------------------------------------

(In reply to comment #5)
> The only remaining detail is the icon, but I'm
> not sure on whether it can be changed via desktop-file-install, or I should
> modify it "manually" (via 'sed', etc...) from the script.

- You can just use sed (at %prep)

------------------------------------------------------------
  This package (cutecom) is APPROVED by mtasaka
------------------------------------------------------------

Comment 7 Jose Luis 2009-02-22 19:35:19 UTC
New Package CVS Request
=======================
Package Name: cutecom
Short Description: A graphical serial terminal.
Owners: jlblanco
Branches: F-9 F-10

Comment 8 Kevin Fenzi 2009-02-22 19:43:18 UTC
cvs done.

Comment 9 Fedora Update System 2009-02-24 20:44:54 UTC
cutecom-0.20.0-3.fc10 has been pushed to the Fedora 10 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.