https://bugzilla.redhat.com/show_bug.cgi?id=1149649 Andrea Musuruane <musuruan@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW Flags|fedora-review? | --- Comment #6 from Andrea Musuruane <musuruan@xxxxxxxxx> --- (In reply to Raphael Groner from comment #5) > 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. Well, you are the first one to think I am not a good reviewer. But I won't go against your wishes. > (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. You know that it was a typo and you know that I got the right SRPM (BTW, I just browsed the correct directory in fedorapeople and got the only SRPM there). Why are you even annoying me with this? > > 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. Because version 0.3.1 is not a PRE-release. It'a RELEASE. Therefore your relase tag is wrong and it MUST NOT start with a 0. > > Better, more descriptive, summary: "Great 2D soccer (sometimes called > > football) game" > > Why is this more descriptive? Again. You are just trying to annoy me. I just tryed to help. > > 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. You don't even know what you are talking about. > > 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. I read SHOULD. Don't you? Anyway the Guidelines suggest the other URL because it is not supposed to change over time. > > In this way you can get rid of the %{ver} macro. > > Why to get rid of it? Because it's no longer needed? > > 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. hicolor-icon-theme owns the directory where you install the icon files. Just type "rpm -ql hicolor-icon-theme" to understand it. > > 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. Sure. Mine too. It was just a friendly suggestion. > > You can use %{_pkgdocdir} instead of %{_docdir}/%{name} > > What will be the benefit? My other packages are not using %{_pkgdocdir}. Again, just a friendly suggestion. %{_pkgdocdir} expands correctly even for older versions of Fedora or RHEL where docs where installed in %{_docdir}/%{name}-%{version}. > > 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. Actually the packager is supposed to do this clean up. > > 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. Yes, it does not complain but the review guidelines tell you to do so because otherwise "your package could fail to generate translation files in the buildroot" > > Moreover, you MUST use the %find_lang macro. > > https://fedoraproject.org/wiki/Packaging:Guidelines#Handling_Locale_Files > > Unclear why this is MUST. Have you bothered to read the Guidelines? Especially the section "Why do we need to use %find_lang?"? https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Why_do_we_need_to_use_.25find_lang.3F > > 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. No, beacuse you don't know if they are overwritten by other flags if you actually cannot read them. > > Incorrect FSF addess should be reported upstream. > > https://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address > > That should not block the approval. And it does not! > > 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. Those are not my wishes. Some of them are MUST items and you must fulfill them before any reviewer can actually approve this review. The others are SHOULD items and they are just suggestions to improve your packager's skills. You should really start listening to other experienced packagers instead of annoying people. Good luck finding another reviewer! -- 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