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: gnome-phone-manager https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198758 ------- Additional Comments From paul@xxxxxxxxxxxx 2006-07-14 06:46 EST ------- (In reply to comment #3) > (In reply to comment #2) > > > - This package used macros. > > > > But did it use them *consistently*? That's what the review guidelines are asking > > to be checked. > Did i missed somthing to check in SPEC? I don't know; I haven't looked at the spec. The review guidelines ask you to check that macros are used consistently, not just that they are used. So you're looking for things like: * Using both $RPM_BUILD_ROOT and %{buildroot} - both expand to the same value; use one or the other but not both in the same spec file * Using %{__rm} but not %{__make} etc. - if the packager is using macros for some commands (e.g. %{__rm}), they should use them in all cases where a macro is available; alternatively, they could just use "rm" instead of "%{__rm}"; the key is consistency > > > - Document files are included like README, NEWS, COPYING, AUTHORS > > > > Did you look to see if there are any other document files in the package that > > might be included, or whether any of the included files don't have anything > > useful to end users of the package? > I didn't get you? Supposing the package contains docs: README, NEWS, COPYING, AUTHOR Did you check to see if there are any other useful documents in the tarball that might be useful to end users? There might be some other .txt files, a docs directory full of useful HTML files, perhaps a TODO file? A reviewer should be looking at a package in much the same way as they would do if they were packaging something themselves, looking to see what would be sensible to include. Also on the subject of documentation, sometimes the documentation provided in tarballs isn't very useful. So sometimes you may see a "NEWS" file that just says to look in the "ChangeLog" file. There is no point in including such a "NEWS" file in the package. That's something a reviewer should be looking at too. > > > * BuildRequires is correct > > > > No, they're not. The package failed to build in mock because of the missing > > buildreqs of perl(XML::Parser) and gettext. > > > I forgot that i added perl(XML::Parser) not AUTHOR. There's also the missing gettext buildreq that caused the package build to fail in mock. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review