Bug 666190

Summary: Reviews Request: libofetion - Library files of Openfetion
Product: [Fedora] Fedora Reporter: Liang Suilong <liangsuilong>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: james.hogarth, package-review
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: AwaitingSubmitter
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-12-04 02:09:09 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 653971    

Comment 1 Liang Suilong 2010-12-30 10:29:45 UTC
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 10:53:22 UTC
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 14:35:47 UTC
(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 22:04:18 UTC
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-04 02:09:09 UTC
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.