https://bugzilla.redhat.com/show_bug.cgi?id=1057991 --- Comment #17 from Dridi Boukelmoune <dridi.boukelmoune@xxxxxxxxx> --- (In reply to Kenjiro Nakayama from comment #16) > Updated > > Updated Spec URL: > http://kenjiro.fedorapeople.org/pkgreview/the_silver_searcher/ > the_silver_searcher.spec > Updated SRPM URL: > http://kenjiro.fedorapeople.org/pkgreview/the_silver_searcher/ > the_silver_searcher-0.21.0-1.20140421git.fc20.src.rpm > Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6761103 > > ~~~ > > [!]: License field in the package spec file matches the actual license. > > Note: Checking patched sources after %prep for licenses. Licenses found: > > "BSD (2 clause)", "Unknown or generated". 25 files have unknown license. > > Detailed output of licensecheck in > > /home/dridi/fedora/_reviews/1057991-the_silver_searcher/licensecheck.txt > > Changed > License: ASL 2.0 > -> License: ASL 2.0 and BSD > > > > [!]: Changelog in prescribed format. > > It means to "0.16-2" in the changelog? > I changed "0.16-2 -> 0.16.0-2". You shouldn't change this in the changelog, upstream's 0.16 exists but not 0.16.0 (see below). Also please ignore my comment about the upstream version 0.21.0 vs 0.21, I don't remember why I put that, and it's clearly wrong. > I checked [1] and compared with my changelogs. But sorry if I misunderstood. You have the following Release tag: Release: 1.%{checkout}%{?dist} You should also have the %{checkout} part in your changelog (but hardcoded). Warning from rpmlint: > the_silver_searcher.x86_64: W: incoherent-version-in-changelog 0.21.0-1 > ['0.21.0-1.20140321git.fc20', '0.21.0-1.20140321git'] 0.21.0-1 -> 0.21.0-1.20140321git I'm also questioning the purpose of the %checkout macro, it doesn't look useful to me. My concern is that it's unrelated to upstream version scheme and that a packager mistake might break the upgrade path. > > [!]: %check is present and all tests pass. > > I did not add %check section for the following reason. > 1. Upstream's test suite is bloken[2] now. > 2. The test suite looks not so important, just creating a few dummy files > and search them. > > If I should include %check section to test them, I will submit patch to > upstream. It's up to you. This one is not a blocker, but I mentioned it because I prefer to run test suites when they exist upstream. > ~~~ > > Thanks, > > Kenjiro You seem to have missed my summary of the issues to fix, I'll put it here again: Issues: ======= - Sources used to build the package match the upstream source, as provided in the spec URL. Note: Upstream MD5sum check error, diff is in /home/dridi/fedora/_reviews/1057991-the_silver_searcher/diff.txt See: http://fedoraproject.org/wiki/Packaging/SourceURL - Is /usr/share/the_silver_searcher/completions/ag.bashcomp.sh needed ? It's identical to /usr/share/bash-completion/completions/ag - Missing the %checkout part in the changelog (is %checkout needed at all?) - BuildRequires of bash-completion should rely on "pkgconfig" instead https://fedoraproject.org/wiki/Packaging:PkgConfigBuildRequires Non issues: (do as you wish!) =========== - Maybe sort the "BuildRequires" alphabetically ? - Consider running the test suite in %check - %defattr present but not needed Best, Dridi -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review