Hide Forgot
Fedora Merge Review: lynx http://cvs.fedora.redhat.com/viewcvs/devel/lynx/ Initial Owner: varekova@redhat.com
I used the checklist from http://www.fedoraproject.org/wiki/JasonTibbitts/ReviewTemplate to look over this package. Someone with more experience should give it a review as well, though. rpmlint output from: http://linux.dell.com/files/fedora/FixBuildRequires/mock-results-core/i386/lynx-2.8.6-2.src.rpm/result/rpmlint.log rpmlint on ./lynx-debuginfo-2.8.6-2.i386.rpm rpmlint on ./lynx-2.8.6-2.src.rpm W: lynx summary-ended-with-dot A text-based Web browser. W: lynx unversioned-explicit-provides webclient rpmlint on ./lynx-2.8.6-2.i386.rpm W: lynx summary-ended-with-dot A text-based Web browser. W: lynx conffile-without-noreplace-flag /etc/lynx.cfg W: lynx conffile-without-noreplace-flag /etc/lynx.lss W: lynx doc-file-dependency /usr/share/doc/lynx-2.8.6/samples/keepviewer /bin/sh W: lynx doc-file-dependency /usr/share/doc/lynx-2.8.6/samples/lynxdump /bin/sh W: lynx doc-file-dependency /usr/share/doc/lynx-2.8.6/samples/oldlynx /bin/sh o The unversioned-explicit-provides webclient issue seems to be standard procedure for web browser packages. firefox, for instance, has the same provides, and gets the same warning from rpmlint o The lack of a noreplace flag on /etc/lynx.cfg is apparently intentional - see the first few lines of that file. o /etc/lynx.lss probably should be flagged as %noreplace o Those doc files are apparently intended to be executable, since they are sample scripts. o source files match upstream: $ sha256sum lynx2.8.6.tar.bz2 41dfc33fcc23295810c3141c614427cca7882ab4e0774e58f6aa9bac9c2586f9 lynx2.8.6.tar.bz2 $ sha256sum lynx2.8.6rel.2.tar.bz2 41dfc33fcc23295810c3141c614427cca7882ab4e0774e58f6aa9bac9c2586f9 lynx2.8.6rel.2.tar.bz2 However, the URL for Source in the spec file is incorrect. The correct URL for the current version is: http://lynx.isc.org/current/lynx2.8.6rel.2.tar.bz2 o package meets naming and versioning guidelines. Looks fine to me. o specfile is properly named, is cleanly written and uses macros consistently. Looks fine to me. o dist tag is present. Nope. Needs to be added o build root is correct. %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) Nope. o license field matches the actual license. Yes. o license is open source-compatible. Yes - GPL V2 in the COPYING file. o latest version is being packaged. Almost - there is a 2.8.6rel.4 available from upstream now. o BuildRequires are proper. Look fine to me. o compiler flags are appropriate. There's some magic in %build - looks like it is for getting the flags right on openssl and Ncurses/mouse support. Someone (other than me) should take a look at this. o %clean is present. Looks fine. o package builds in mock ( ). Yes. o package installs properly Yes. o debuginfo package looks complete. I'm not sure. o rpmlint is silent. See the warnings at the top ^ o final provides and requires are sane: Yes. o %check is present and all tests pass: No. o no shared libraries are added to the regular linker search paths. Looks ok to me. o owns the directories it creates. Ok. o doesn't own any directories it shouldn't. Ok. o no duplicates in %files. Ok. o file permissions are appropriate. Yes, except might want to chmod -x the sample scripts that are located in /usr/share/doc/lynx-2.8.6/samples/ (rpmlint complains about them) o no scriptlets present. Ok. o code, not content. Ok. o documentation is small, so no -docs subpackage is necessary. Ok. o %docs are not necessary for the proper functioning of the package. Ok. o no headers. Ok. o no pkgconfig files. Ok. o no libtool .la droppings. Ok. o not a GUI app. Well, no, it isn't. :-)
Thanks for your comments. The fixed package is lynx-2.8.6-3.fc7. o /etc/lynx.lss probably should be flagged as %noreplace -> fixed However, the URL for Source in the spec file is incorrect. The correct URL for the current version is: http://lynx.isc.org/current/lynx2.8.6rel.2.tar.bz2 -> lynx2.8.6rel.2.tar.bz2 - is not stable version o dist tag is present. Nope. Needs to be added -> added o build root is correct. %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) -> fixed o latest version is being packaged. Almost - there is a 2.8.6rel.4 available from upstream now. -> it is not stable version lynx 2.8.6 is te last stable version o %check is present and all tests pass: No. ->There are no tests in lynx package o file permissions are appropriate. Yes, except might want to chmod -x the sample scripts that are located in /usr/share/doc/lynx-2.8.6/samples/ (rpmlint complains about them) -> I think this files should be leaved as executable.
Nope, files in %doc can't be executable.
Please could you sent me some link to the documentation which supports your opinion?
Sorry Ivana, I was wrong. Jesse (Keating) told me, that it's possible to ship files with +x, when no new dependencies are introduced by these files.
No problem. Thanks for your time. Could you please set fedora-review flag to + if you agree the package fulfil all requirements, or attach some comment with the list of problems which should be fixed if you see some? Thank you very much.
Robin, could you please look at less and approved this review request or if you see any reason why you wdon't want to aproved it here. Thanks
Oh, sorry Ivana - everything looks fine now.
Thanks
Can this ticket be closed now? It appears approval was given and fedora-review + is assigned.
I'm confused about this part of the package review process as well. According to: http://fedoraproject.org/wiki/PackageReviewProcess "5. Once a package is flagged as fedora-review + (or -), the Reviewer's job is done. "
The approval was granted so I'm closing this bug.