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=486302 --- Comment #32 from Lubomir Rintel <lkundrak@xxxxx> 2009-04-19 09:16:21 EDT --- Uff, this seems to need a lot of work to be accepted. Sorry the review took so long. 1.) Don't require perl modules by package name: BuildRequires: perl-Test-Harness Requires: perl-Pod-Simple ... This would be better: BuildRequires: perl(Test::Harness) ... 2.) Source1 is useless: BuildRequires: ed Source1: reduce_rpmlint_err.tar.gz Do what the scripts do inline. It might be a lot easier to use sed instead of ed. 3.) Subpackage buildrequire %package docs ... BuildRequires: perl %package tools ... BuildRequires: ed Makes no sense. Please don't define BRs in subpackages. Furthermore, perl is in default buildgroup, thus can be ommited. 4.) Why would tools require pkgconfig? %package tools ... Requires: pkgconfig 5.) This always evaluates to false if test "%{_vendor}" = "suse" please remove it 6.) No architecture independence %ifarch %{ix86} x86_64 ... %else # PowerPC ... %endif Remove this, or explain. 7.) %{_smp_mflags} not used make make parrot_utils make installable make html Either use them or explain why you don't. 8.) No useless comments please #make install DESTDIR=$RPM_BUILD_ROOT ... #rm -rf $RPM_BUILD_ROOT/%{_docdir}/parrot # for Solaris? 9.) 10.) Don't strip anything %{__strip} %{RPM_PAR_LIB_DIR}dynext/*.so Remove this 11.) Bad comment rm -rf $RPM_BUILD_ROOT%{_usr}/share/doc/parrot # necessary for SuSE Well, eh, SUSE? 12.) Use macros for directories where appropriate --lex=/usr/bin/flex \ --yacc=/usr/bin/yacc \ ... rm -rf $RPM_BUILD_ROOT%{_usr}/share/doc/parrot # necessary for SuSE %{_bindir}, %{_datadir}, etc... 13.) Compiler generates .so-s executable # Force permissions on shared versioned libs so they get stripped. find $RPM_BUILD_ROOT%{_libdir} -type f -name '*.so.*' -exec chmod 755 {} \; Is this necessary? Explain if yes. 14.) Enable test suite # make test < /dev/null # %{?_with_fulltest:make fulltest < /dev/null} # make test || : # %{?_with_fulltest:make fulltest || :} 15.) Comment patches and submit them upstream? Patch0: parrot-install_files.patch What does this do? Was it submitted upstream? 16.) Provides not sane parrot-tools: perl(DB) Perl debugger? I guess not. perl(File::Which) = 0.05 You embed File::Which in /usr/lib/parrot/1.0.0/tools/lib/File/Which.pm You should not. Depend on perl-File-Which package instead. parrot-docs: perl(A) perl(B) You should not provide these. 17.) Doc shouldn't depend on perl: parrot-docs requires these: perl(strict) perl(warnings) -- 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@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review