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=231861 --- Comment #11 from Michal Hlavinka <mhlavink@xxxxxxxxxx> 2009-10-29 11:49:54 EDT --- (In reply to comment #10) > Is this package still ready for review? somehow :o) > (And shouldn't it be a Merge Review?) definitely > How can it be that it has status Assigned but isn't assigned to anybody? afaict, it seems it was assigned to someone whose account has been closed > My first impression is that it looks like it haven't been dressed up for examn > and could use some polishing before a final review. sure, I've inherited this package and it required a *lot* of polishing. But after a few rounds it got lower and lower in my todo-list, especially because there was no reviewer. > Some brief comments: > > It seems like most (all?) of the code now is licensed announcement-BSD-ish, so > the License and the comments about it are a bit misleading. fixed > > The spec is quite complex and verbose and IMHO not easy to read. I agree > The spec contains comments left over from the Invoca version. fixed > _perlhack variable seems to be unused since 7.3 - Red Hat, not Fedora! yes, this was leftover, I've removed all perlhack ifdefs some time ago removed > > There are manu variables and configuration options. Are they necessary and > used? this is what I can't even guess actually. I'd definitely like to get rid of all those switches, but I don't want to break it for someone... Well, I've just though about removing them in new rawhide and wait if someone complains. and since devel is future rawhide now, I've removed them > > The %file specs are very explicit and verbose. Is that intentional and > necessary? (And %{_contribdir} is listed twice.) second _contribdir removed it seems to me there is quite a lot of space for improvement since attributes do not need to be specified twice (install in %install and %files), I'll look at this. > Rpmlint says > 6 packages and 1 specfiles checked; 20 errors, 61 warnings. > Some of the warnings might be invalid, but some of them definitely should be > adressed before review. I've lowered the number a little for now > The spec has 30 sources and 15 patches without any indication if they have been > pushed upstream. Afaik they were rejected some sources are additional modules/tools that upstream is not interested in ---------------------------------- This is first round and definitely not finished. Just to show you there is someone on the other end. I'll continue with this on monday ----------------------------------- changes were only commited, not tagged yet, you find actual (not finished) spec in cvs -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review