Bug 666190 - Reviews Request: libofetion - Library files of Openfetion
Reviews Request: libofetion - Library files of Openfetion
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
Unspecified Unspecified
low Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
AwaitingSubmitter
:
Depends On:
Blocks: 653971
  Show dependency treegraph
 
Reported: 2010-12-29 08:06 EST by Liang Suilong
Modified: 2015-12-03 21:09 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2015-12-03 21:09:09 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Comment 1 Liang Suilong 2010-12-30 05:29:45 EST
I update spec files and build it again. 

SRPM: http://liangsuilong.fedorapeople.org/libofetion-2.1.0-2.fc13.src.rpm
SPEC: http://liangsuilong.fedorapeople.org/libofetion.spec

The changelog is here:

- Clean up BuildRequires
- Add serveral documents
- Change Home URL

Koji Result: http://koji.fedoraproject.org/koji/taskinfo?taskID=2693775
Comment 2 Michael Schwendt 2010-12-30 05:53:22 EST
Same issues as in bug 653971 (OpenFetion review):

* Unowned directories:

  %{_datadir}/openfetion/
  %{_datadir}/openfetion/resource/

http://fedoraproject.org/wiki/Packaging:UnownedDirectories


> Summary:        Library files of Openfetion

Rather terse and unclear, IMO. It's likely that a lib* package contains library files, and obviously somebody who would search for "fetion" will find libofetion. Is this library attractive to other people, too? What parts of the Fetion protocol does it implement? E.g.

| Summary: Instant messaging client library for the Fetion protocol V4


> %description
> Fetion is an instant messaging (IM) client developed 
> by China Mobile, [...]

But this is libofetion for OpenFetion, so the description ought to start with explaining what _this_ package contains. A few sentences later it could comment on the relationship with the original Fetion or the Fetion Protocol V4.


> %setup -q -n %{name}-%{version}

Option -n %{name}-%{version} is the default. No need to add it.
Comment 3 Liang Suilong 2010-12-30 09:35:47 EST
(In reply to comment #2)
> Same issues as in bug 653971 (OpenFetion review):
> 
> * Unowned directories:
> 
>   %{_datadir}/openfetion/
>   %{_datadir}/openfetion/resource/
> 
> http://fedoraproject.org/wiki/Packaging:UnownedDirectories
> 

I have suggested move city.xml and province.xml from %{_datadir}/openfetion/resource/ to %{_datadir}/libofetion/resource/. Openfetion and libofetion will not own the same directory. And I will fix the issues in bug 653971. 


> 
> > Summary:        Library files of Openfetion
> 
> Rather terse and unclear, IMO. It's likely that a lib* package contains library
> files, and obviously somebody who would search for "fetion" will find
> libofetion. Is this library attractive to other people, too? What parts of the
> Fetion protocol does it implement? E.g.
> 
> | Summary: Instant messaging client library for the Fetion protocol V4
> 
> 
> > %description
> > Fetion is an instant messaging (IM) client developed 
> > by China Mobile, [...]
> 

I have rewrote a new summary and description. You can read it in next version. 

> But this is libofetion for OpenFetion, so the description ought to start with
> explaining what _this_ package contains. A few sentences later it could comment
> on the relationship with the original Fetion or the Fetion Protocol V4.
> 
> 
> > %setup -q -n %{name}-%{version}
> 
> Option -n %{name}-%{version} is the default. No need to add it.

OK. I fix it. 

I am waiting author's update. He says that he will push a new version.
Comment 5 Jason Tibbitts 2012-04-24 18:04:18 EDT
Just some notes, as I am looking over old review tickets.

You have License: GPLv2+, but I see the openssl license included as well.  I checked and the program does appear to be linked against openssl, which is not permitted by the GPL.  However, fortunately I also checked the COPYING file and see the standard openssl linking exception there.  This means that you should have:
  License: GPLv2+ with exceptions

This review is pretty old, and the package includes a snapshot which must be quite out of date now.  Can you update with a more recent version?

Your Release: tag indicates you're packaging a snapshot, but the Source0: URL just points to a release tarball.  Which is it?

Otherwise this package looks pretty good at first glance.  It builds fine and romlint doesn't have much to complain about:

  libofetion.src:16: W: mixed-use-of-spaces-and-tabs
   (spaces: line 1, tab: line 16)
It's nice to be consistent about tabs or spaces.

  libofetion.src: W: invalid-url Source0:
   http://ofetion.googlecode.com/files/libofetion-2.1.0.tar.gz
   HTTP Error 404: Not Found
This is just google code being broken.

  libofetion.x86_64: E: incorrect-fsf-address
   /usr/share/doc/libofetion-2.1.0/COPYING
  libofetion.x86_64: E: incorrect-fsf-address
  /usr/share/doc/libofetion-2.1.0/LICENSE
The FSF has moved; this needs to be reported upstream (if it isn't already fixed in the current version).

It is not necessary to include BuildRoot, %clean or the first line of %install any longer (unless you want this package to be used for EPEL5).  It is not necessary to include %defattr(-,root,root,-) at all, for Fedora or EPEL.
Comment 6 James Hogarth 2015-12-03 21:09:09 EST
The associated application bug was closed a long time back as per the stalled review process.

As this cannot occur without that also closing this.

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