Bug 230710 - Review Request: boolstuff - Disjunctive Normal Form boolean expression library
Review Request: boolstuff - Disjunctive Normal Form boolean expression library
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-03-02 08:44 EST by Patrice Dumas
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-05-13 10:50:05 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Patrice Dumas 2007-03-02 08:44:20 EST
Spec URL: http://www.environnement.ens.fr/perso/dumas/fc-srpms/boolstuff.spec
SRPM URL: http://www.environnement.ens.fr/perso/dumas/fc-srpms/boolstuff-0.1.10-1.src.rpm

Description: 
This library contains an algorithm that converts a boolean expression
binary tree into the Disjunctive Normal Form.  The NOT operator
is supported.
Comment 1 Mamoru TASAKA 2007-04-29 13:34:29 EDT
For 0.1.10-1:

? Subpackages
  - Please justify to split booldnf "subpackage" from
    boolstuff "main" package. Multilib effect or something?
    ( I don't know about multilib issue well)
    Usually splitting packages only addes complexity and
    confusion.

    Also, if you still want to split booldnf package, please
    reconsider the name of main package (usually, these
    type of package should be named as "-libs").

* Timestamps
  - This software installs some files which are not modified
    during build stage (e.g. header files, man files) 
    and keeping timestamps on these files is recommended.
    For this package, the following method works.
-------------------------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="%{__install} -p"
docdir=%{_datadir}/tmpdocdir
-------------------------------------------------------------------

? File entry
-------------------------------------------------------------------
/usr/include/boolstuff-0.1/boolstuff/BoolExpr.cpp
-------------------------------------------------------------------
  - Why is this file needed? (well, actually this file is called from
    BoolExpr.h, however is it correct?)

? documentation
  - For -devel subpackage:
-------------------------------------------------------------------
%doc examples/example.cpp examples/test-booldnf.pl examples/non-string-nodes.cpp
-------------------------------------------------------------------
    - Any reason you want to remove example/ directory?

===================================================================
!!
   I would appreciate it if you would review any of my ruby modules
   related review requests
   (bug 237380 and bug 237379 , both needed by alexandria < bug 237382 >
    bug 237381 is being reviewed by Chris Weyl)
Comment 2 Pierre Sarrazin 2007-04-29 22:46:57 EDT
Version 0.1.11 is out. I fixed the installation targets so that the example
programs do not get installed anymore.

http://sarrazip.com/dev/boolstuff.html
Comment 3 Mamoru TASAKA 2007-05-04 04:02:15 EDT
ping?
Comment 4 Patrice Dumas 2007-05-05 06:48:33 EDT
Sorry Mamoru and Pierre, I am very busy at this time and there was
a security issue in dap-server. I'll try to progress on the week end
but I cannot promise.

About the review of the ruby packages, I am sorry, I don't know
anything about ruby, I wouldn't make a good review ;-(. But tell
me if you have other reviews. Lately I stopped completely 
looking at the 'extras' reviews and focus on 'core' reviews,
but currently I have very little time.
Comment 5 Patrice Dumas 2007-05-06 14:16:22 EDT
* Subpackages:

I think that booldnf and the library should be separated. 
In general only one should be needed. The library for 
C/C++ and booldnf to use it as a command, for example from 
other languages. It also solves a multilib issue.

Now for the package names, it could also be:

booldnf in boolstuff
library in boolstuff-libs

Just tell what you prefer.

* I can't see what is wrong with
/usr/include/boolstuff-0.1/boolstuff/BoolExpr.cpp?

* Other issues are fixed in

http://www.environnement.ens.fr/perso/dumas/fc-srpms/boolstuff-0.1.11-1.fc7.src.rpm
http://www.environnement.ens.fr/perso/dumas/fc-srpms/boolstuff.spec

- update to 0.1.11
- use a directory for in-source docs
Comment 6 Mamoru TASAKA 2007-05-07 12:42:14 EDT
(In reply to comment #5)
> * Subpackages:
> Now for the package names, it could also be:
>
> booldnf in boolstuff
> library in boolstuff-libs
> 
> Just tell what you prefer.

I won't disagree with your naming for this package.

> * I can't see what is wrong with
> /usr/include/boolstuff-0.1/boolstuff/BoolExpr.cpp?

I asked just because I don't usually see files named "*.cpp"
as header files.. But it seems okay for this package.

> * Other issues are fixed in
> boolstuff-0.1.11-1.fc7.src.rpm

Okay!!
--------------------------------------------------
  This package (boolstuff) is accepted by me
--------------------------------------------------

(In reply to comment #4)
> About the review of the ruby packages, I am sorry, I don't know
> anything about ruby, I wouldn't make a good review ;-(. But tell
> me if you have other reviews. 

Well, now 3 ruby modules packages are under review by 
Chitlesh GOORAH. I am still waiting for the following 3 packages:

bug 237382 (alexandria) - blocked by bug 237380 (perhaps
    this will soon be accepted). While alexandira is a 
    GNOME "application", it heavily depends on ruby...
bug 233424 (perl-mecab) and bug 233425 (mecab-java) ..
    perl/java binding for MeCab. these don't depend on ruby
Comment 7 Patrice Dumas 2007-05-08 13:34:21 EDT
New Package CVS Request
=======================
Package Name: boolstuff 
Short Description: Disjunctive Normal Form boolean expression library
Owners: pertusus[ AT ]free.fr
Branches: FC-6 EL-4 EL-5
InitialCC: sarrazip[ AT ]sarrazip.com
Comment 8 Patrice Dumas 2007-05-08 13:38:30 EDT
I know that obfuscating mail addresses is problematic for those
who create the branches, but otherwise bots could collect the
mail addresses.

Thanks for the review Mamoru, I'll happily review perl-mecab, but 
I can't review java-mecab since I don't know java any better than 
ruby. I'll see if I can review alexandria once the blocker is accepted.
Comment 9 Warren Togami 2007-05-09 16:26:00 EDT
It is a futile effort.  Also note, your address is not obfuscated in the spec files.
Comment 10 Patrice Dumas 2007-05-09 16:35:07 EDT
Right, bit I could have obfuscated it and there was also Pierre 
address.
Comment 11 Mamoru TASAKA 2007-05-13 10:16:11 EDT
Please close when rebuild is done.
Comment 12 Patrice Dumas 2007-05-13 10:49:46 EDT
Thanks for the review Mamoru, indeed it is built. I had a look 
at alexandria, but I am not sure that I can review it given my
low ruby skills. I'll see what I can do.

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