Fedora Merge Review: psmisc http://cvs.fedora.redhat.com/viewcvs/devel/psmisc/ Initial Owner: kzak
I reviewed this one. Just a few things: rpmlint on ./psmisc-22.2-5.src.rpm W: psmisc summary-ended-with-dot Utilities for managing processes on your system. W: psmisc macro-in-%changelog _includedir Please use the %{dist} in release BuildRequires should require gettext rather than gettext-devel Otherwise, it looks good.
Andy, please reassign tickets back to the requester and switch the fedora-review flag to -, if you deny the request +, if you approve it. See http://www.fedoraproject.org/wiki/WarrenTogami/ReviewWithFlags for more info.
Ruben, sorry for not following procedure. I should have explained that I'm not sponsored yet, so I do not have access to reassign tickets.
Fixed. Update to psmisc-22.3-1.fc7. Thanks for your review.
Adding current owner. Karel, what is the status here? I see you are the previous owner. Any objection to my taking ownership of the bug, and completing the review?
Jon, I am the new maintainer of psmisc. You can take the ownership and complete the review.
Excellent. I'll get right on it.
Full review is stellar. One think I might ask, and it's not worth a rebuild, but could you comment in the spec on the purpose and upstream status of the patches? Just got added to the Guidelines and I think it's a good practice.
Ping?
Ping again?
Daniel, can you please follow up to Jon's proposals? -- Also: * Not necessary to define globally when used only for make export CFLAGS="$RPM_OPT_FLAGS -D_GNU_SOURCE" also not sure whether is "-D_GNU_SOURCE" necessary, shouldn't $RPM_OPT_FLAGS take care of everything? * don't mix variable and macro style https://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS * %defattr(-,root,root) A little bit old-school to me. Use %defattr(-,root,root,-) * point URL field to http://sourceforge.net/projects/psmisc/ the present one is useless (bogus content there)
hello, I made the suggested .spec changes: * added purpose comments to all patches * fixed URL * fixed %defattr as for the others: > * Not necessary to define globally when used only for make configure and autoreconf doesn't use those? I'm not sure... > * don't mix variable and macro style as far as I can see, there's only variable style for those two variables output: http://people.fedoraproject.org/~dnovotny/psmisc.review.spec
(In reply to comment #12) > [...] > output: > http://people.fedoraproject.org/~dnovotny/psmisc.review.spec Looks good to me. (Don't forget tu bump changelog entry when in CVS, please.) Jon: Are you satisfied? Will you approve it, or should I?
I'm happy. Commit. APPROVED. Thanks everyone!