Bug 503847 - Review Request: paperbox - A GTK tracker based document browser
Review Request: paperbox - A GTK tracker based document browser
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Susi Lehtola
Fedora Extras Quality Assurance
:
Depends On: 503256
Blocks:
  Show dependency treegraph
 
Reported: 2009-06-02 20:47 EDT by Gareth John
Modified: 2009-09-11 19:04 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-09-11 19:04:54 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
susi.lehtola: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)
gcc44 patch, works on F-12 (544 bytes, text/plain)
2009-07-28 05:02 EDT, Mamoru TASAKA
no flags Details

  None (edit)
Description Gareth John 2009-06-02 20:47:41 EDT
Spec URL: http://garethsrpms.googlecode.com/files/paperbox.spec
SRPM URL: http://garethsrpms.googlecode.com/files/paperbox-0.4.2-1.fc10.src.rpm
Description: Paperbox is a document browser. It lets you nicely view your 
ebooks, office and text documents and organise them by tags.
Relying on Tracker, it is able to instantly discover all 
documents on your desktop, and present them in a convenient 
way. Tags and other metadata are shared across all 
Tracker-based applications.
Comment 1 Susi Lehtola 2009-06-03 06:51:39 EDT
- Judging from the Paperbox install page the buildrequires should be

gtkmm-utils-devel boost-devel gtkmm24-devel glibmm24-devel libglademm-devel goocanvas-devel libgnomeui-devel tracker-devel dbus-glib-devel

Now you have gtkmm24-devel and tracker-devel listed twice.


- Also, you need Requires: hicolor-icon-theme for dir ownership.
Comment 2 Mamoru TASAKA 2009-06-06 03:56:53 EDT
Well, at least BR: libgnomeui-devel is missing:
http://mtasaka.fedorapeople.org/Review_request/paperbox/MOCK-paperbox.log

! By the way, as seen above you can still try mockbuild even if
  BR contains not-yet-approved packages by setting local yum
  repository.
Comment 3 Gareth John 2009-06-09 13:30:16 EDT
Spec URL: http://garethsrpms.googlecode.com/files/paperbox.spec
SRPM URL: http://garethsrpms.googlecode.com/files/paperbox-0.4.2-1.fc10.src.rpm

There is the latest revision. I have added and tidy the BR, I am reasonably sure that they are correct, I'm in the middle of moving at the minute and am aware i have not done much in a few days (week) and wanted to post. This should be complete however as times are hectic. Thanks Mamoru for the log was helpful. Have not studied it greatly but will do so. Tomorrow or at least sometime this week.
Comment 4 Mamoru TASAKA 2009-06-09 13:41:41 EDT
Just a note:
- Please change the release number of your spec/srpm every time
  you modify them to avoid confusion.
Comment 5 Gareth John 2009-06-09 16:03:05 EDT
I apologise i do change the revision i just copied the wrong link, here is the latest one

SRPM URL: http://garethsrpms.googlecode.com/files/paperbox-0.4.2-2.fc10.src.rpm
Comment 6 Susi Lehtola 2009-06-09 17:57:35 EDT
OK, I'll have a look after you have fixed the remaining issues in gtkmm-utils.
Comment 7 Mamoru TASAKA 2009-06-11 15:39:18 EDT
(Note that I don't intend to do full-review on this package, I
 am just trying to check if this package correctly builds)


0.4.2-2.fc10 won't build on F-12 because of another issue:
http://mtasaka.fedorapeople.org/Review_request/paperbox/MOCK-paperbox-x86_64-2.fc10.log

With the following patch, at least on F-12 i386/x86_64 this package
builds (again I just checked if this package builds or not)
http://mtasaka.fedorapeople.org/Review_request/paperbox/paperbox-0.4.2-g++44.patch

http://mtasaka.fedorapeople.org/Review_request/paperbox/MOCK-paperbox-i586-modified-2.fc10.log
http://mtasaka.fedorapeople.org/Review_request/paperbox/MOCK-paperbox-x86_64-modified-fc10.log
Comment 8 Susi Lehtola 2009-07-05 06:39:23 EDT
ping?
Comment 9 Susi Lehtola 2009-07-22 08:38:08 EDT
ping again John
Comment 10 Gareth John 2009-07-27 11:12:23 EDT
Hello no movement as per your comment #6 Jussi. Have no updated gtkmm-utils as you no doubt know.
Comment 11 Susi Lehtola 2009-07-27 17:05:34 EDT
(In reply to comment #10)
> Hello no movement as per your comment #6 Jussi. Have no updated gtkmm-utils as
> you no doubt know.  

Right.

**

There's no need to run
 --remove-key Encoding                                   \

you can
 --remove-category=GNOME
as it doesn't serve any purpose in Fedora

and the Applications category is obsolete, so drop
 --add-category Applications                             \
(keep the --remove-category=Application)

**

You're running desktop-file-install, so you should give as source file
 data/paperbox.desktop
instead of
 $RPM_BUILD_ROOT%{_datadir}/applications/paperbox.desktop

**

The package builds for Fedora 10, but not for Fedora 11. This will have to be fixed. Too bad Mamoru has already removed the necessary patch.

**

You are missing the %posttrans phase of the icon cache update.

**

The license is GPLv2+ not GPLv2 (see source code).
Comment 12 Mamoru TASAKA 2009-07-28 05:02:50 EDT
Created attachment 355380 [details]
gcc44 patch, works on F-12

(In reply to comment #11)
> The package builds for Fedora 10, but not for Fedora 11. This will have to be
> fixed. Too bad Mamoru has already removed the necessary patch.

Ah, I thought paperbox was already imported...
Only tested on F-12 i686 (again I just checked if the srpm builds
or not)
Comment 13 Susi Lehtola 2009-08-16 05:12:20 EDT
ping
Comment 14 Gareth John 2009-08-18 11:40:12 EDT
http://garethsrpms.googlecode.com/files/paperbox-0.4.3-4.fc11.src.rpm
http://garethsrpms.googlecode.com/files/paperbox.spec

No rpmlint issues, changed as per recommendation. I also change to new upstream source 0.4.3, I am now running fc11 and compiles fine without any use of patches i assume this will be the same for fc12 ?
Comment 15 Susi Lehtola 2009-08-19 03:41:11 EDT
Yes, seems to build fine in rawhide. Reset the release to 1, as you've updated the version.
Comment 16 Susi Lehtola 2009-08-20 05:49:49 EDT
rpmlint output is clean.


MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK

MUST: The spec file for the package is legible and macros are used consistently. NEEDSWORK
- Don't mix '%{name}' and 'paperbox' in %files. Choose one and stick with it.
- You can use longer lines in %description and fit it in 5 lines.

MUST: The package must be named according to the Package Naming Guidelines. OK
MUST: The spec file name must match the base package %{name}. OK
MUST: The package must be licensed with a Fedora approved license and meet the  Licensing Guidelines. OK
MUST: The License field in the package spec file must match the actual license. OK
MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK
MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. OK
MUST: Optflags are used and time stamps preserved. OK
MUST: Packages containing shared library files must call ldconfig. N/A
MUST: A package must own all directories that it creates or require the package that owns the directory. OK
MUST: Files only listed once in %files listings. OK
MUST: Debuginfo package is complete. OK
MUST: Permissions on files must be set properly. OK
MUST: Clean section exists. OK
MUST: Large documentation files must go in a -doc subpackage. N/A
MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. OK
MUST: Header files must be in a -devel package. N/A
MUST: Static libraries must be in a -static package. N/A
MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A
MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. N/A
MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. N/A
MUST: Packages does not contain any .la libtool archives. N/A

MUST: Desktop files are installed properly. NEEDSWORK
- Drop --add-category="X-Fedora", it isn't used.

MUST: No file conflicts with other packages and no general names. OK
MUST: Buildroot cleaned before install. OK
SHOULD: %{?dist} tag is used in release. OK
SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK
SHOULD: The package builds in mock. OK
Comment 18 Susi Lehtola 2009-08-25 06:24:19 EDT
You didn't update the spec file, it is still at 0.4.3-4. (It took me a while to remember that I told you to reset the release.)

**

I'd divide the description as

Paperbox is a document browser. It lets you nicely view your ebooks, office
and text documents and organise them by tags. Relying on Tracker, it is able 
to instantly discover all documents on your desktop, and present them in a
convenient way. Tags and other metadata are shared across all Tracker-based
applications.

as IMHO it looks a bit cleaner this way. With this comment the package is

APPROVED
Comment 19 Gareth John 2009-08-27 11:43:04 EDT
New Package CVS Request
=======================
Package Name: paperbox
Short Description: A GTK tracker based document browser
Owners: gljohn
Branches: F10 F11
InitialCC: n/a
Comment 20 Jason Tibbitts 2009-08-28 10:37:05 EDT
Branches are named "F-10", "F-11", etc.  I've fixed that up.

CVS done.

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