[Bug 595637] Review Request: qoauth- Qt-based C++ library for OAuth authorization scheme

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=595637

Thomas Spura <tomspur at fedoraproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |tomspur at fedoraproject.org
             Blocks|                            |595638
         AssignedTo|nobody at fedoraproject.org    |tomspur at fedoraproject.org
               Flag|                            |fedora-review+

--- Comment #1 from Thomas Spura <tomspur at fedoraproject.org> 2010-06-20 09:56:42 EDT ---
I have a slow internet connection atm.

Will do a full review, when I can download the qt stuff this week...

A few comments for now:
- Is there a reason why you use %{?isa}? I don't think leaving them out will
cause problems, but maybe I'm wrong...?
This makes only sense, when using e.g:
%ifarch %ix86
Requires: %{name}.(i?86|athlon|geode)
%endif
%ifarch x86_64 amd64 ia32e
Requires: %{name}.(x86_64|amd64|ia32e)
%endif

Or is this common in KDE?
(This is my first look at a KDE/qt package ever^^)

- You aren't consistend with tabbing:
  4 Name:           qoauth
  5 Version:                1.0.1
  6 Release:                0.1.%{gitdate}git%{githash}%{?dist}
  7 Summary:                Qt-based C++ library for OAuth authorization scheme
  8 Group:          System Environment/Libraries
  9 License:                LGPLv2+
 10 URL:                    http://github.com/ayoy/qoauth

Would be more readable otherwise.

- checksum does not match (which is excpected, because of the timestamps):
  diff -r is clean

- name ok
- koji successfull: http://koji.fedoraproject.org/koji/taskinfo?taskID=2259994
- rpmlint ok:
$ rpmlint ./qoauth-1.0.1-0.1.20100525gitec7e4d5.fc13.src.rpm ./x86_64/qoauth-*
qoauth.src:11: W: macro-in-comment %{version}
qoauth.src:11: W: macro-in-comment %{name}
qoauth.src:11: W: macro-in-comment %{version}
qoauth.src:14: W: macro-in-comment %h
qoauth.src:14: W: macro-in-comment %h
qoauth.src: W: no-cleaning-of-buildroot %install
qoauth.src: W: no-buildroot-tag
qoauth.src: W: invalid-url Source0: qoauth-ec7e4d5.tar.bz2
4 packages and 0 specfiles checked; 0 errors, 8 warnings.

macro-in-comment could be avoided when using s/%/%%/g.

- libs correctly packaged
- no static libs
- no *.la
- parallel make is there
- BR/R ok
- doc is not too big, to need to split in a subpackage

#############################################################################

Only cosmetic issues: macro-in-comment + tabbing + %{?isa}.

#############################################################################

APPROVED

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.