Hide Forgot
Spec URL: http://www.squirrel.nl/pub/xfer/EekBoek.spec SRPM URL: http://www.squirrel.nl/pub/xfer/EekBoek-1.04.02-1.fc11.src.rpm Description: EekBoek is a bookkeeping package for small and medium-size businesses. Unlike other accounting software, EekBoek has both a command-line interface (CLI) and a graphical user-interface (GUI, currently under development). Furthermore, it has a complete Perl API to create your own custom applications. EekBoek is designed for the Dutch/European market and currently available in Dutch only. An English translation is in the works (help appreciated). The spec file is non-trivial but well commented. The spec file and rpms are rpmlint free with the exception of two files that are not utf8. This is intentional, these files need to be latin1. Results of last koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1094851 This is the third package I submit for review, and I'm looking for a sponsor.
Some random notes: * Licensing - First of all, from what file(s) can we judge that this software is under Artistic 2.0? * Requires/Provides - What package provides "%{name}-db"? (are the rebuilt binary rpms installable for you?) - By the way, if you make -db-{sqlite,postgresql} subpackages have "Provides: %{name}-db = %{version}", AFAIK "yum install EekBoek" will always install EekBoek and EekBoek-db-sqlite because yum usually gives higher priority to shorter name. Is this what you expect? Usually I don't recommend to add the virtual Requires on the "main" package like this (because with this yum will always choose one) and suggest to write a note which tells that admins have to install either of the packages providing database backends afterwards by themselves. * BuildRequires - "BuildRequires: glibc-common" is redundant. * Duplicate directories - Since both database backend subpackages have "Requires: %name", listing the following directories in the subpackages is not needed. Just move to the main package. --------------------------------------------------------------- %dir %{ebshare} %dir %{ebshare}/lib %dir %{ebshare}/lib/EB %dir %{ebshare}/lib/EB/DB --------------------------------------------------------------- * %changelog - As it is useful when using Fedora CVS, I recommend to put one line between %changlog entries like: --------------------------------------------------------------- %changelog * Fri Jan 30 2009 Johan Vromans <jvromans@squirrel.nl> - 1.04.02-1 - Adapt to Fedora guidelines * Sun Jan 26 2009 Johan Vromans <jvromans@squirrel.nl> - 1.04.02 - Remove QUICKSTART. * Sat Jul 19 2008 Johan Vromans <jvromans@squirrel.nl> - 1.03.90 - Remove debian stuff - Don't use unstable. ---------------------------------------------------------------
ping?
Stay tuned, I'll get back ASAP.
Thanks for your kind and constructive feedback. Most items are trivially fixed, which I will do in a next release. The requires/provides "%{name}-db" seems to be confusing, which may be an indication that the intention is not clear. Or wrong. As you may have guessed, EekBoek supports both SQLite and PostgreSQL as database backend (and probably more to come). It is tempting to bundle the SQLite backend with the main package (that is what Debian does) but I personally do not like having to install a package that one doesn't need. What would be a better approach for this?
(In reply to comment #4) > Thanks for your kind and constructive feedback. Most items are trivially fixed, > which I will do in a next release. > > The requires/provides "%{name}-db" seems to be confusing, which may be an > indication that the intention is not clear. Or wrong. > > As you may have guessed, EekBoek supports both SQLite and PostgreSQL as > database backend (and probably more to come). It is tempting to bundle the > SQLite backend with the main package (that is what Debian does) but I > personally do not like having to install a package that one doesn't need. > > What would be a better approach for this? I would say that bundling SQLite backend to main package is undesired solution. In such case I recommend - not to use virtual "Provodes/Requires: %{name}-db" - write some text named "README.Fedora", for example to inform that - BekBook needs one of the database backends - Currently Fedora supports SQLite and PostgreSQL - The admin has to choose either of them and install the choosed one by "yum install BekBook-db-<choosed backend>" by him/herself and add to the "%description" of BekBook package some sentence like "Please read README.Fedora"
Thanks for the feedback. Unless someone comes up with a more elegant solution I'll consider this approach for the next release.
As discussed already, I'll review this and your other 2 perl package submissions, and once they are all approved, I'll sponsor you. I've done a full review of this package and I fully agree with Mamoru, please fix the points he has addressed and then I'll aprove this. As for the license tag, I've checked the various .pm sources, and the correct license tag would be: License: GPL+ or Artistic
(In reply to comment #5) > I would say that bundling SQLite backend to main package is > undesired solution. However, in https://bugzilla.redhat.com/show_bug.cgi?id=483286#c6, Hans de Goede writes: > I know this is optional but in Fedora we always try to enable as much > features in packaes as possible, even if this drags in a few additional > dependencies. Following this practice, it would be preferred to include the SQLite driver in the main package, especially since SQLite is relatively small and already a core part of many packages. This would result in a working installation for people that do not read READMEs.
(In reply to comment #8) > (In reply to comment #5) > > I would say that bundling SQLite backend to main package is > > undesired solution. > > However, in https://bugzilla.redhat.com/show_bug.cgi?id=483286#c6, Hans de > Goede writes: > > > I know this is optional but in Fedora we always try to enable as much > > features in packaes as possible, even if this drags in a few additional > > dependencies. > > Following this practice, it would be preferred to include the SQLite driver in > the main package, especially since SQLite is relatively small and already a > core part of many packages. > > This would result in a working installation for people that do not read > READMEs. +1 Regards, Hans
(In reply to comment #8) > (In reply to comment #5) > > I would say that bundling SQLite backend to main package is > > undesired solution. > > However, in https://bugzilla.redhat.com/show_bug.cgi?id=483286#c6, Hans de > Goede writes: > > > I know this is optional but in Fedora we always try to enable as much > > features in packaes as possible, even if this drags in a few additional > > dependencies. > > Following this practice, it would be preferred to include the SQLite driver in > the main package, especially since SQLite is relatively small and already a > core part of many packages. > > This would result in a working installation for people that do not read > READMEs. Actually SQLite is rather small and I agree with you.
Spec URL: http://www.squirrel.nl/pub/xfer/EekBoek.spec SRPM URL: http://www.squirrel.nl/pub/xfer/EekBoek-1.04.03-1.fc11.src.rpm Description: EekBoek is a bookkeeping package for small and medium-size businesses. Unlike other accounting software, EekBoek has both a command-line interface (CLI) and a graphical user-interface (GUI, currently under development). Furthermore, it has a complete Perl API to create your own custom applications. EekBoek is designed for the Dutch/European market and currently available in Dutch only. An English translation is in the works (help appreciated). The spec file is non-trivial but well commented. The spec file and rpms are rpmlint free with the exception of two files that are not utf8. This is intentional, these files need to be latin1. All comments from the reviews are applied. SQLite is now included as default database backend. Source is upgraded to new upstream version. Results of last koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1303621
Some notes: * %define -> %global - For some reasons (especially due to rpm's "unexpected" behaviour with nested macros), Fedora now suggests to use %global instead of %define: https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define * Including additional documents as Source - When including additional documents, please add them as Sources instead of making patch, like: ----------------------------------------------------------------- Source1: README.postgres ..... ..... %prep %setup -q %patch0 -p0 -b .script cp -p %SOURCE1 . ----------------------------------------------------------------- (Also see the below comments about timestamps) * Dependency between subpackages - Usually the dependency between packages rebuilt from the same srpm is strict EVR (Epoch-Version-Release) specific (not only version specific): https://fedoraproject.org/wiki/Packaging/Guidelines#Requiring_Base_Package ! %setup - "-n %{name}-%{version}" is the default of %setup, so "%setup -q" is enough here. * Timestamps - When using "cp" or "install" commands, use "-p" option to keep timestamps on installed files: https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps * Permission ----------------------------------------------------------------- %{__find} blib/lib ! -type d -printf "%{__install} -m 0444 %p %{buildroot}%{ebshare}/lib/%%P\n" | sh -x ----------------------------------------------------------------- - Usually these files should have 0644 permission. * %files ----------------------------------------------------------------- # Collect the list of files. Basically we include all files except # the EB/DB (database specific) modules. ( cd %{buildroot}; %{__find} ./%{ebshare} -type d -printf "%%%%dir %{ebshare}/%%P\n"; %{__find} ./%{ebshare} -type f -printf "%{ebshare}/%%P\\n" | %{__grep} -v "lib/EB/DB/.*pm" ) > .files # Include the SQLite driver. echo '%{ebshare}/lib/EB/DB/Sqlite.pm' >> .files ----------------------------------------------------------------- - The following is much simpler: ----------------------------------------------------------------- %files %defattr(-,root,root,-) %doc ....... %dir %{_sysconfdir}/%{lcname} %config(noreplace) %{_sysconfdir}/%{lcname}/%{lcname}.conf %{ebshare}/ %exclude %{ebshare}/lib/EB/DB/Postgres.pm %{_bindir}/ebshell %{_mandir}/man1/* %files db-postgresql %defattr(-,root,root,-) %doc README.postgres %{ebshare}/lib/EB/DB/Postgres.pm ----------------------------------------------------------------- * Documents - Usually the file "INSTALL" is for people who want to install the package by themselves and not needed for rpm users.
Mamoru thanks for the input, Johan what Mamoru said :)
Spec URL: http://www.squirrel.nl/pub/xfer/EekBoek.spec SRPM URL: http://www.squirrel.nl/pub/xfer/EekBoek-1.04.03-2.fc11.src.rpm Description: EekBoek is a bookkeeping package for small and medium-size businesses. Unlike other accounting software, EekBoek has both a command-line interface (CLI) and a graphical user-interface (GUI, currently under development). Furthermore, it has a complete Perl API to create your own custom applications. EekBoek is designed for the Dutch/European market and currently available in Dutch only. An English translation is in the works (help appreciated). The spec file is non-trivial but well commented. The spec file and rpms are rpmlint free with the exception of two files that are not utf8. This is intentional, these files need to be latin1. All comments from review #12 are applied, although I must remark that I do not see why an Epoch is needed. The link is all but helpful, and some further reading reveals that 'Epoch should be avoided if possible'... Results of last koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1310184
(In reply to comment #14) > All comments from review #12 are applied, although I must remark that I do not > see why an Epoch is needed. The link is all but helpful, and some further > reading reveals that 'Epoch should be avoided if possible'... Ah, I didn't say Epoch must be introduced, what I said is that -db-postgresql subpackage should have "%{name} = %{version}-%{release}" which is exact EVR specific, not just "%{name} = %{version}".
Thanks for the clarification. I'll throw out the epoch related things.
Spec URL: http://www.squirrel.nl/pub/xfer/EekBoek.spec SRPM URL: http://www.squirrel.nl/pub/xfer/EekBoek-1.04.03-3.fc11.src.rpm Description: EekBoek is a bookkeeping package for small and medium-size businesses. Unlike other accounting software, EekBoek has both a command-line interface (CLI) and a graphical user-interface (GUI, currently under development). Furthermore, it has a complete Perl API to create your own custom applications. EekBoek is designed for the Dutch/European market and currently available in Dutch only. An English translation is in the works (help appreciated). The spec file is non-trivial but well commented. The spec file and rpms are rpmlint free with the exception of two files that are not utf8. This is intentional, these files need to be latin1. Same as 1.04.03-2, with Epoch: removed. Results of last koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1313959
(In reply to comment #17) > Spec URL: http://www.squirrel.nl/pub/xfer/EekBoek.spec > SRPM URL: http://www.squirrel.nl/pub/xfer/EekBoek-1.04.03-3.fc11.src.rpm Looks fine to me now, approved! Johan, can you please apply for packager group member ship in the account system (if you haven't done that already), and tell me your FAS username, then I'll sponsor you. Regards, Hans
New Package CVS Request ======================= Package Name: EekBoek Short Description: Bookkeeping package for small and medium-size businesses Owners: sciurius Branches: F-10 F-11 InitialCC:
cvs done.
EekBoek-1.04.03-3.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/EekBoek-1.04.03-3.fc11
EekBoek-1.04.03-3.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/EekBoek-1.04.03-3.fc10
EekBoek-1.04.03-3.fc10 has been pushed to the Fedora 10 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update EekBoek'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-3972
EekBoek-1.04.03-3.fc11 has been pushed to the Fedora 11 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update EekBoek'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-4342
EekBoek-1.04.03-3.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.
EekBoek-1.04.03-3.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.