https://bugzilla.redhat.com/show_bug.cgi?id=1149649 Raphael Groner <projects.rg@xxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|musuruan@xxxxxxxxx |nobody@xxxxxxxxxxxxxxxxx --- Comment #5 from Raphael Groner <projects.rg@xxxxxxxx> --- Andrea, I am sorry to have to speak that out, but your review is not of good quality. I don't get the connection to the official review guidelines. Therefore I would like to find another reviewer, I think to have the right to not have to accept any first coming random reviewer, cause there could be several reviewers doing just informal review hints. Of course, the approval could be done only by one reviewer that thinks all review issues are fixed. (In reply to Andrea Musuruane from comment #1) > Correct SRPM URL seem to be: > https://raphgro.fedorapeople.org/review/tuxfootball/tuxfootball-0.3.1-0.1. > fc20.src.rpm I don't understand why you do a review for an URL I did not post. Maybe it was a typo of mine, maybe not. It's only an assumption from your side that may be correct. So that's the first point why you should not accept a formal review. > I'm using this one to perform the review. > > Release tag is wrong. You are using a pre-release tag but tuxfootball-0.3.1 > has been released. > https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Release_Tag Where's written that 0.1 is not allowed for a pre-release of the spec file itself? My idea was to use 0.x for preofficial spec files, 1 will then be when it's allowed to go into SCM. > Better, more descriptive, summary: "Great 2D soccer (sometimes called > football) game" Why is this more descriptive? > License is wrong. It is "GPLv2+" (note the +) not "GPLv2". License GPLv2 includes any upstream compatibility. But I can use + additionally, if you think this is so important. > Source0 URL is wrong. It should be: > http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz > https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net Should or must? My link is documented upstream and works for a correct download of the source tarball. > In this way you can get rid of the %{ver} macro. Why to get rid of it? > Guidelines require to "Requires:" a package when you install a file into a > directory that the package does not own. You install icons into > /usr/share/icons/hicolor and therefore you MUST "Requires: > hicolor-icon-theme". > https://fedoraproject.org/wiki/Packaging: > Guidelines#File_and_Directory_Ownership Wrong. My package does not need any icons from hicolor-icon-theme. The folder could be owned by several packages, like it's also so for any other folders. > You must use trademarks in a way that is not ambiguous. Avoid phrasing like > "similar to" or "like". Therefore avoid using "Amco's Kick Off" and > "Sensible Software's Sensible Soccer" in description. > https://fedoraproject.org/wiki/Packaging:Guidelines#summary > > You can improve the description adding: > The gameplay is designed to be quick, responsive and fun. You are always > in control of the player closest to the ball. The ball is controlled via > two different kick buttons - one for pass, and one for shoot. Aftertouch > can be applied to shots by quickly pressing and holding the direction you > want the ball to bend towards. Pushing in the opposite direction to what > you kicked the ball makes it raise into the air, pushing in the same > direction as the ball makes it dip towards the ground. The original description was taken from upstream. > You MUST use scriptlets to update the icon cache to ensure that the Desktop > files are able to load the included icon files. > https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache ok > Because the data package is build from the same source as the main package, > there is no real gain in splitting them apart. The data content is noarch. I think doing that is generally a good practice, and I have already done that for other approved packages, too. > You can use %{_pkgdocdir} instead of %{_docdir}/%{name} What will be the benefit? My other packages are not using %{_pkgdocdir}. > Remove ChangeLog from docs. It is irrelevant documentation (it is about > source code commits) and it should not be packaged. > https://fedoraproject.org/wiki/Packaging:Guidelines#Documentation The question is not if it's irrelevant. It's more about if upstream wishes to have ChangeLog shipped as official documentation. So better include it than being punished for not distributing it. I don't feel responsible as a package for cleaning up the tarball I got from upstream. > The package uses gettext for translations so you should add "BuildRequires: > gettext". I have tested with help from mock (koji build). It does not complain. > Moreover, you MUST use the %find_lang macro. > https://fedoraproject.org/wiki/Packaging:Guidelines#Handling_Locale_Files Unclear why this is MUST. > Compilation is not verbose. Therefore it is not possible to check the > compiler flag used. Invoke make like "make %{?_smp_mflags} V=1" The right compiler flags are ensured by using %make macro. > Incorrect FSF addess should be reported upstream. > https://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address That should not block the approval. > I'll perform a deeper analysis after these issues are fixed. As I can not fulfill all of your wishes, I don't see why you want to and think to be still able to do a formal review. -- 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