https://bugzilla.redhat.com/show_bug.cgi?id=1057991 --- Comment #18 from Kenjiro Nakayama <knakayam@xxxxxxxxxx> --- 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.fc20.src.rpm Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6764985 ~~~ > 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 changed back "0.16.0-2 -> 0.16-2". > You should also have the %{checkout} part in your changelog (but hardcoded). ... (snip) ... > 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. I reconsidered about %checkout part, and I think it doesn't need it anymore. Previous version might need to update by the checkout, but it doesn't need anymore. So I deleted %checklog. > You seem to have missed my summary of the issues to fix, I'll put it here again: Yes... Sorry. > 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 Updated > - Is /usr/share/the_silver_searcher/completions/ag.bashcomp.sh needed ? > It's identical to /usr/share/bash-completion/completions/ag Deleted, I also think it is not necessary. - Missing the %checkout part in the changelog (is %checkout needed at all?) Deleted %checkout part in the spec. > - BuildRequires of bash-completion should rely on "pkgconfig" instead > https://fedoraproject.org/wiki/Packaging:PkgConfigBuildRequires Changed to "BuildRequires: pkgconfig(bash-completion)" > Non issues: (do as you wish!) > =========== > - Maybe sort the "BuildRequires" alphabetically ? Yes, I sorted. > - Consider running the test suite in %check > - %defattr present but not needed Deleted. ~~~ Thanks, -- 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