Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=476832





--- Comment #10 from Mamoru Tasaka <mtas...@ioa.s.u-tokyo.ac.jp>  2009-01-29 
14:49:35 EDT ---
Some notes:

* License
  - License tag should be GPLv2+.

* %description place
  - Would you move the place of %package and %description to the
    upper side of %prep?

* Documents
  - I guess it is better that QUICKSTART.mysql is installed
    in -mysql subpackage and QUICKSTART.postgres in -pgsql subpackage.
  - For HOWTO:
--------------------------------------------------------------------
install -Dp -m 644 %{SOURCE1} %{_builddir}/%{name}-%{base_version}
--------------------------------------------------------------------
    * Move this to %prep section, if "HOWTO" file is not to be modified
      at %build.
    * By the way is HOWTO also needed for subpackages?

* Current working directory
--------------------------------------------------------------------
make install DESTDIR=%{_builddir}/%{name}-%{base_version}/mysql
--------------------------------------------------------------------
  - make install DESTDIR=$(pwd)/mysql is simpler.
    (at %build, %install (and %check) stage, the working directory
     is %{_builddir}/%{name}-%{base_version} in this case)

    For "install -Dp ....", even
--------------------------------------------------------------------
install -Dp ./$database%{_bindir}/mydnscheck
%{buildroot}%{_bindir}/mydnscheck-$database
--------------------------------------------------------------------
    can be used (for DESTDIR=, usually this should be absolute path)

* CFLAGS
--------------------------------------------------------------------
make CFLAGS="%{optflags}" %{?_smp_mflags}
--------------------------------------------------------------------
  - Would you check if this "CFLAGS=%{optflags}" is really needed?
    %configure sets CFLAGS environment variable and configure/Makefiles
    created from recent autotools honor this CFLAGS.

* autotool recall
--------------------------------------------------------------------
   644  + make 'CFLAGS=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables' -j8
   645  cd . && /bin/sh /builddir/build/BUILD/mydns-1.2.8/missing --run
aclocal-1.10 -I m4
   646  /builddir/build/BUILD/mydns-1.2.8/missing: line 46: aclocal-1.10:
command not found
   647  WARNING: `aclocal-1.10' is missing on your system.  You should only
need it if
   648           you modified `acinclude.m4' or `configure.ac'.  You might want
   649           to install the `Automake' and `Perl' packages.  Grab them from
   650           any GNU archive site.
   651  cd . && /bin/sh /builddir/build/BUILD/mydns-1.2.8/missing --run
autoconf
--------------------------------------------------------------------
  - Here autotools are called automatically after calling make.
    This usually means that timestamps on some files are wrong.
    Please modify the timestamps of those to prevent autotool
    recall here.

* Directory structure
--------------------------------------------------------------------
install -Dp -m 644 contrib/admin.php %{buildroot}%{_datadir}/admin.php
--------------------------------------------------------------------
  - I don't think that putting files under %_datadir (not under the
    subdirectory of %_datadir) is a good idea. This should be
    moved to %{_datadir}/%{name}, for example.

* gettext .po file
  - This package has gettext .po files under po/ directory and
    actually "make install" installs compiled .mo files.
    Please include gettext .mo file in binary rpm (and use %find_lang)

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

_______________________________________________
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review

Reply via email to