Fedora Merge Review: pam_krb5 http://cvs.fedora.redhat.com/viewcvs/devel/pam_krb5/ Initial Owner: nalin
1) rpmlint reported pam_krb5.src: W: spelling-error %description -l en_US pam -> map, Pam, pan pam_krb5.src: W: spelling-error %description -l en_US pluggable -> plug gable, plug-gable, plugged pam_krb5.src: W: invalid-url URL: https://fedorahosted.org/pam_krb5/ <urlopen error [Errno -2] Name or service not known> pam_krb5.i686: W: spelling-error %description -l en_US pam -> map, Pam, pan pam_krb5.i686: W: spelling-error %description -l en_US pluggable -> plug gable, plug-gable, plugged pam_krb5.i686: W: invalid-url URL: https://fedorahosted.org/pam_krb5/ <urlopen error [Errno -2] Name or service not known> 2 packages and 0 specfiles checked; 0 errors, 6 warnings. ==> This can be ignored 2)timestamps should be preserved.Use INSTALL="install -p" when installing to preserve timestamps. 3) I will suggest this package to follow current packaging guidelines and remove buildroot, %clean section and cleaning of build root in %install 4) Should follow https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make
Created attachment 434148 [details] pam_krb5-spec-cleanup.spec please add changelog entry
Also, As per https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags , please add comment in spec why you need extras flags.
Thank you for picking up this review! (In reply to comment #1) > 2)timestamps should be preserved.Use INSTALL="install -p" when installing to > preserve timestamps. Done. > 3) I will suggest this package to follow current packaging guidelines and > remove buildroot, %clean section and cleaning of build root in %install Okay, done. > 4) Should follow > https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make Done. (In reply to comment #3) > Also, > As per https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags , > please add comment in spec why you need extras flags. I think at the time (and this would have been quite a while ago) libtool wasn't doing the right thing and building the contents of the module with -fPIC, but it seems to be doing the right things now. Removing the CFLAGS tweaks. The changes are made in upstream git if you'd like to review them: http://git.fedorahosted.org/git/?p=pam_krb5.git;a=blob;f=pam_krb5.spec
Thanks. New spec looks good. Just minor thing, if adding my name please use just "Parag AN" as when seen plain text spec file in git it showed garbage characters in parenthesis :) Please commit in devel and build new package and will approve this review.
Whoops, sorry about that. I can't read that script, so I tried to copy and paste from bugzilla. Should be all checked-in and built now.
Thanks for the changes. Package is APPROVED now.