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=807017 --- Comment #10 from Juan Hernández <juan.hernandez@xxxxxxxxxx> 2012-04-12 06:28:42 EDT --- (In reply to comment #9) > Some comments on the spec file: Thank you very much for the review. > - Why does the spec file contain a huge license section at the > top? I think you should avoid this, unless there is some pressing > reason for a specific license on the spec file itself. No need for that, I will remove it. > - py_site_pkgs uses %define, should almost certainly be using %global. You are right, missed that one. > - These seem to be unnecessary. I would remove them and use the programs > directly. > %global __getent /usr/bin/getent > %global __groupadd /usr/sbin/groupadd > %global __useradd /usr/sbin/useradd > %global __usermod /usr/sbin/usermod I will do that. > - The whole business of splitting the spec file into different *.inc > files ... I can't see this getting past a Fedora review, so I suggest > that you don't do it. I thought that splitting the spec in several files could make it easier to maintain, but I agree with you that it is not common practice. I will revert that change. > - There's some pretty funky stuff going on in scripts, such as backing > up directories before they are removed by RPM (and thereby bypassing > the whole purpose of RPM). What is the purpose of this and how much > of this can be avoided? Note that scripts are (a) the thing most likely > to fail during RPM installation and (b) the hardest thing to debug because > it happens on someone else's computer, so it's in your interest to > make scripts as simple/non-existent as possible. I will review that and remove as much as possible. Do you see any other important blocker for this package? -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review