Bug 485636 - Review Request: cutecom - A GUI application for serial port communications
Review Request: cutecom - A GUI application for serial port communications
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-15 13:23 EST by Jose Luis
Modified: 2009-02-24 15:44 EST (History)
3 users (show)

See Also:
Fixed In Version: 0.20.0-3.fc10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-02-24 15:44:59 EST
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 Jose Luis 2009-02-15 13:23:08 EST
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 14:23:30 EST
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 03:55:22 EST
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 12:47:09 EST
Ah... you can't assign what you submitted to yourself.
Comment 4 Mamoru TASAKA 2009-02-19 13:10:35 EST
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 17:59:04 EST
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 11:42:38 EST
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 14:35:19 EST
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 14:43:18 EST
cvs done.
Comment 9 Fedora Update System 2009-02-24 15:44:54 EST
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.