Bug 967659 - Review Request: robojournal - cross-platform journal/diary tool
Review Request: robojournal - cross-platform journal/diary tool
Status: CLOSED DEFERRED
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
AwaitingSubmitter
:
: 1009703 (view as bug list)
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2013-05-27 16:56 EDT by Will Kraft
Modified: 2015-07-21 09:56 EDT (History)
4 users (show)

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


Attachments (Terms of Use)

  None (edit)
Description Will Kraft 2013-05-27 16:56:02 EDT
Spec URL: http://pagewizardwebdesign.com/stuff/robojournal.spec and http://www.pagewizardwebdesign.com/stuff/robojournal-doc.spec

SRPM URL: http://pagewizardwebdesign.com/stuff/robojournal-0.4.1-1.fc18.src.rpm and http://pagewizardwebdesign.com/stuff/robojournal-doc-0.4.1-1.fc18.src.rpm

Description: 
RoboJournal is a cross-platform journal/diary tool written in Qt/C++.
Right now, RoboJournal only supports MySQL but support for SQLite 
(and possibly Postgres) will be added in future releases. RoboJournal 
runs on Windows and Linux.

Fedora Account System Username: pwizard

I have had difficulty building this package in Koji (several failed attempts) but I have been able to successfully build RPMS locally. I would appreciate some help because this is the first time I've packaged for Fedora and some things are still new to me.
Comment 1 Will Kraft 2013-05-27 16:57:45 EDT
This is my first package and I am seeking a sponsor.
Comment 2 Eduardo Echeverria 2013-05-28 19:23:25 EDT
Hi Will
See https://fedoraproject.org/wiki/Package_Review_Process#Contributor
Please put in the "Blocks" field, the tag FE-NEEDSPONSOR
Comment 4 Michael Schwendt 2013-06-17 04:33:51 EDT
* Try to do a self-review of your package with the help of the following page:
  https://fedoraproject.org/wiki/Packaging:ReviewGuidelines

  [ https://fedoraproject.org/wiki/Join_the_package_collection_maintainers ]
  [ https://fedoraproject.org/wiki/Category:Package_Maintainers ]


* Run "rpmlint -I" on all packages, the src.rpm *and* the built rpms. Apply fixes for obvious errors/warnings, ignore false positives, preferably comment on what rpmlint reports.


* A brief look at the spec file:


> Summary:            Free journal software for everyone

Summary: Cross-platform journal/diary tool

would do a better job at summing up what the package offers.


> BuildRequires:      qt, qt-assistant, qt-mysql, qt-devel, qt-webkit,
> qt-webkit-devel, patch

1) "patch" is available in the minimum build environment already and need not be specified as BuildRequires:
https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2

2) Double-check the others for redundancies. At least "qt" and "qt-webkit" will be available as dependencies of qt-devel and qt-webkit-devel already.


> # Apply standard Fedora patch so the app compiles properly
> patch Makefile < fedora_build.patch

Hmmm, this is fragile. First of all, it would be more normal to apply patches in %prep (after %setup) via %patchX and to add them as PatchX tags in the spec file. But since the patch file is included in the tarball, applying it manually is acceptable. Secondly, applying a patch manually in %build like it is done here breaks --short-circuit -bc rpmbuilds. So, apply patches in %prep.


> Requires:           qt, qt-assistant, qt-mysql, qt-webkit

https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires


> make 

https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make


> strip robojournal

Don't strip any files yourself. That breaks generation of -debuginfo packages. rpmlint will notice that, too.

https://fedoraproject.org/wiki/Packaging:Guidelines#Debuginfo_packages


> #install files manually because "make install" doesn't work
> with rpmbuild in this case.

_What_ "doesn't work"?



> %post
> mandb -p

To be deleted. Packages don't do that.


> %clean
> make distclean

To be deleted. "make distclean" is also not what %clean is used for.

https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean


> %{_datadir}/applications/robojournal.desktop

You need to _use_ desktop-file-utils inside the spec file when packaging .desktop files:
https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files


> %{_datadir}/menu/robojournal

The directory /usr/share/menu doesn't exist yet in Fedora filesystem packages, and this package doesn't include it either. What is it used for? Isn't it specific to Debian based systems?

$ repoquery --whatprovides /usr/share/menu
luckybackup-0:0.4.7-3.fc19.x86_64


> %{_mandir}/man7/robojournal.7.gz

%{_mandir}/man7/robojournal.7*   would be cleaner, since it allows for a changed/customised/dropped compression technique.
Comment 5 Michael Schwendt 2013-10-28 06:29:30 EDT
*** Bug 1009703 has been marked as a duplicate of this bug. ***
Comment 6 Miroslav Suchý 2015-07-21 09:56:55 EDT
Closing due long inactivity. Feel free to reopen if you want to continue.

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