Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: bogl - a graphics library and an Unicode terminal emulator
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=214124
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2006-11-09 12:55 EST ------- Well, for my viewpoint:
1. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines : * Licensing * Rpmlint - Well, rpmlint complains about no-url-tag. It seems that this package is currently maintained by debian people. I recommend that you add the URL which shows that this package is now maintained by debian.
Also, you should add "debian/copyright" to main package AND -bterm package as -bterm package does not require main package.
* Tags - Use %?dist tag. - I recommend using %_datadir instead of /usr/share.
* Compiler flags - fedora specific compilation flags are not passed. ------------------------------------------------------ + make DESTDIR=/var/tmp/bogl-0.1.18-12-root-mockbuild libdir=/usr/lib install cc -O2 -g -D_GNU_SOURCE -Wall -D_GNU_SOURCE -DBOGL_CFB_FB=1 -DBOGL_VGA16_FB=1 -o bdftobogl.o -c bdftobogl.c cc bdftobogl.o bogl.o bogl-font.o bogl-cfb.o bogl-pcfb.o bogl-tcfb.o bogl-vga16.o -o bdftobogl ./bdftobogl helvB10.bdf > helvB10.c cc -O2 -g -D_GNU_SOURCE -Wall -D_GNU_SOURCE -DBOGL_CFB_FB=1 -DBOGL_VGA16_FB=1 -o helvB10.lo -fPIC -c helvB10.c ./bdftobogl helvB12.bdf > helvB12.c cc -O2 -g -D_GNU_SOURCE -Wall -D_GNU_SOURCE -DBOGL_CFB_FB=1 -DBOGL_VGA16_FB=1 -o helvB12.lo -fPIC -c helvB12.c ./bdftobogl helvR10.bdf > helvR10.c ...... ------------------------------------------------------
* Timestamps I always recommend to keep timestamps of - text files (including header files) - image files - etc to show clearly when they are created/modified. For the case of this package, keeping timestamps of header files in -devel package is preferable. For this case, the usual method 'make INSTALL="install -c -p" install cannot be used. Try like: sed -i -e 's|install|install -p|g' Makefile or so.
2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines : = Nothing.
3. Other things I have noticed: * For bogl-bterm - Well, Japanese people (including me) always complain about bterm as this software (bterm) is not useful for non-root users because *it seems* bterm requires device access right to /dev/tty0 . (I have not checked the whole source code of bterm and perhaps it is impossible for me). What do you think of this?
+ kon2 (which was in Core till FC5, however it was removed for FC6) used to setuid on kon binary. + For jfbterm, I decided to use console.perms method. (through long discussion with the original maintainer) This method allows the FIRST user to use CUI frame buffer to gain device access right. + or should we leave this as it was?
+++++++++++++++++++++++++++++++++++++++++++++++++++++++ In my opinion, the following are okay.
= %find_lang There is no gettext mo files so %find_lang macro is not needed, this is correct.
= /usr/share/bogl This is correctly owned by bogl-bterm
= /usr/include/bogl This is correctly owned by bogl-bterm
= You are using: "Requires: bogl = %{epoch}:%{version}-%{release}" This is correct when using epoch.
= I can't see any resaon why you need to use the epoch tag For this package, epoch is needed as Epoch was already used when this package was in Fedora Core.
= source files license issue: Well, surely some of the source files are not explicitly licensed, however, for now I trust that these are licensed under GPL accroding to debian/copyright.
= bogl does not use autoconf/automake In my opinion, autoconf/automake should not be used unless it is unavoidable and there is no problem for this package.
= There is a lot of compile warnings Well, compilation warnings should be avoided as well as possible, however my opinion is this is not a blocker as long as the warnings are not _crucial_ . I maintain xscreensaver, of which the compilation warning canNOT be avoided because of gtk2 "bug".
Jules, any comments?