Bug 223588 - Review Request: rudeconfig - C++ library for manipulating config files
Summary: Review Request: rudeconfig - C++ library for manipulating config files
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Ralf Corsepius
QA Contact: Fedora Package Reviews List
Depends On:
TreeView+ depends on / blocked
Reported: 2007-01-20 08:17 UTC by Matt Flood
Modified: 2007-11-30 22:11 UTC (History)
0 users

Clone Of:
Last Closed: 2007-02-22 17:08:41 UTC
wtogami: fedora-cvs+

Attachments (Terms of Use)

Description Matt Flood 2007-01-20 08:17:19 UTC
Spec URL: http://www.rudeserver.com/config/download/rudeconfig.spec
SRPM URL: http://www.rudeserver.com/config/download/rudeconfig-5.0.4-1.src.rpm

rudeconfig is a C++ library for reading, writing and 
manipulating configuration/.ini files.

Thanks in advance for the review.

Comment 1 Ralf Corsepius 2007-01-20 09:32:05 UTC
Congrats, a pretty clean package!

Disable building the static library and I'll approve it.

Minor issue: The website doesn't mention version 5.0.4 yet. ATM, the tarball
only seems to be accessible when directly accessing it (I could access and
verifiy it)

A note to upstream (seemingly you): Your configure scripts supports building
bz2-compressed tarballs - It might we worth considering to using the bz2'ed
tarball instead of the gziped versions. Spares a couple of bytes on disks/CDs etc.

Comment 2 Ralf Corsepius 2007-01-20 12:27:30 UTC

- Disable building the static lib (%configure --disable-static)
  and reflect this change where needed.

- Remove commented out duplicate %configure.
  rpm expands comments which causes %configure to be invoked twice.
  If you want to use % in comments you must use %% instead of %.


- Minor typo in changelog:
  Fri Sep 2 2005 .. server.com?

  This should read ...server.com>

Comment 3 Ralf Corsepius 2007-02-01 07:52:45 UTC

Comment 4 Matt Flood 2007-02-01 17:14:33 UTC
Ping... Thank you so much.    I will fix and bump today!!

Comment 5 Matt Flood 2007-02-01 18:38:40 UTC
Once again, thank you for your review.

I have disabled building the static library.  I also removed the 
corresponding %{_libdir}/*.a from the relevant %files section.

I removed the commented out duplicate %configure 
(thank you for the information regarding rpm expansion of comments)

I fixed the typo that was found in the comment section.  I addressed the same 
typo in the the ChangeLog file.

As per your advice, I have switched to using the bz2'ed tarball instead of the 
gzip'ed ones. (thanks for that advice)

I have increased the release number from 1 to 2.  I apologize in advance if my 
assumption to bump that number was wrong.

I have added comments reflecting the major changes for this release in NEWS, 
ChangeLog, and the specfile.

The URLs for the updated files are:

Spec URL: http://www.rudeserver.com/config/download/rudeconfig.spec
SRPM URL: http://www.rudeserver.com/config/download/rudeconfig-5.0.4-2.src.rpm

I am in the process of making version 5.0.4 "visibly" available on the 

Thank you!

Comment 6 Michael Schwendt 2007-02-01 19:08:14 UTC
> %{_includedir}/rude/config.h

The directory itself is not included!

Comment 7 Matt Flood 2007-02-01 19:48:15 UTC
I am sorry, but I do not understand the problem.  
Looking at the following extract of the current spec file:

--------- current extract of spec file-----
%files devel


Do you mean that I have ommitted the directory, and should add something like:


Or are you saying that I should omit the directory "rude" and just have:


Or is the blank line that I (accidently) introduced below %doc and before
%{_includedir}/config.h a problem (maybe messed up a diff result)?

Or something else that is way over my head - admittedly, my brain is a bit
slodgy today..

Thank you:)

Comment 8 Michael Schwendt 2007-02-01 20:08:54 UTC
The former.

And that's either:

| %files devel
| %defattr(-,root,root,-)
| %{_includedir}/rude/
| %{_libdir}/*.so
| %{_mandir}/man3/*


| %files devel
| %defattr(-,root,root,-)
| %dir %{_includedir}/rude
| %{_includedir}/rude/config.h
| %{_libdir}/*.so
| %{_mandir}/man3/*

When you list the binary rpm, you want to see a "drwxr-xr-x" entry
for "/usr/include/rude".

The difference between

| %dir %{_includedir}/rude
| %{_includedir}/rude/config.h


| %{_includedir}/rude/

is that the latter includes the directory "rude" and the entire
tree below it. On the contrary, with %dir you can include specific
directories, but you need to specify any files in addition to that.

Comment 9 Matt Flood 2007-02-01 23:43:56 UTC
Thank you so much for explaining that to me.  I have used the 

%dir %{_includedir}/rude

The URLs for the updated files are:

Spec URL: 


Thanks again!

Comment 10 Ralf Corsepius 2007-02-02 01:37:20 UTC
Packaging-wise *-3.src.rpm seems fine.

There remains one (upstream) issue:

uses INCLUDED_CONFIG_H as header guard.

This isn't necessarily a clever choice, because INCLUDED_CONFIG_H is a pretty
generic name which likely to conflict with other package's defines.

I'd recommend to use _RUDE_CONFIG_H or similar.

Comment 11 Matt Flood 2007-02-02 04:19:07 UTC
Thank you.  I have modified the config.h to use the more unique include guard:


As a result, I bumped the package version up to 5.0.5 and set the release to 1


Comment 12 Ralf Corsepius 2007-02-02 05:01:48 UTC

(In reply to comment #11)
> Thank you.  I have modified the config.h to use the more unique include guard:

OK, if you prefer it this way :)

BTW: I can't find your name in owners.list - Could it be you need to a sponsor?

Comment 13 Matt Flood 2007-02-02 18:37:31 UTC
Yes, this is my first review.  I failed to set the FE-NEEDSPONSOR block, my
apologies. Should I add it now?

Comment 14 Ralf Corsepius 2007-02-03 15:13:23 UTC
Not necessary - I am going to sponsor you.

Please proceed with step 10 as described on

Comment 15 Ralf Corsepius 2007-02-20 11:29:48 UTC
Matt? What is the status of this package?

The package doesn't seem be imported into FE's cvs, yet.

Comment 16 Matt Flood 2007-02-20 16:33:32 UTC
My apologies, 
  I am currently hung up on the "Identify Yourself as the Owner of the 
Package". It seems that "CVSAdminProcedure" is different now than it was 2 
weeks ago when I went out of town for a conference.  At the time, I had to 
modify some wiki page to get the cvs directories built.  Now I cannot find 
that wiki and this seems to be the first time I have seen the 
CVSAdminProcedure, which I did not do. Nevertheless, the cvs directories were 
created, and I have just imported the package, but I don't seem to have 
requested FC5 and FC6  branches.  Is this something I need to do as a comment 
here?  Or are FC5 and FC6 not applicable for development libraries?

Comment 17 Matt Flood 2007-02-21 17:09:37 UTC
New Package CVS Request
Package Name: rudeconfig
Short Description: C++ library for manipulating configuration and .ini files
Owners: matt@rudeserver.com
Branches: FC-6 FC-5
InitialCC: mflood@fuelcellstore.com

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