[Bug 1149649] Review Request: tuxfootball - funny soccer game
https://bugzilla.redhat.com/show_bug.cgi?id=1149649 Raphael Groner changed: What|Removed |Added Blocks||201449 (FE-DEADREVIEW) Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=201449 [Bug 201449] FE-DEADREVIEW -- Reviews stalled due to lack of submitter response should be blocking this bug. -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1149649] Review Request: tuxfootball - funny soccer game
https://bugzilla.redhat.com/show_bug.cgi?id=1149649 Raphael Groner changed: What|Removed |Added Status|NEW |CLOSED Resolution|--- |WONTFIX Last Closed||2014-10-23 18:22:08 --- Comment #7 from Raphael Groner --- Thanks for killing this review request. -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1149649] Review Request: tuxfootball - funny soccer game
https://bugzilla.redhat.com/show_bug.cgi?id=1149649
Andrea Musuruane changed:
What|Removed |Added
Status|ASSIGNED|NEW
Flags|fedora-review? |
--- Comment #6 from Andrea Musuruane ---
(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 abo
[Bug 1149649] Review Request: tuxfootball - funny soccer game
https://bugzilla.redhat.com/show_bug.cgi?id=1149649
Raphael Groner changed:
What|Removed |Added
Assignee|musur...@gmail.com |nob...@fedoraproject.org
--- Comment #5 from Raphael Groner ---
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.
> Compilatio
[Bug 1149649] Review Request: tuxfootball - funny soccer game
https://bugzilla.redhat.com/show_bug.cgi?id=1149649 Andrea Musuruane changed: What|Removed |Added Assignee|nob...@fedoraproject.org|musur...@gmail.com Whiteboard|NotReady| --- Comment #4 from Andrea Musuruane --- The NotReady is only used not to make other possible reviewer lose time by looking at a reviews that are not yet ready. I already assigned this package review to myself (so you already had found a reviewer!) and I already started the review process. The review process is an interative process. I asked you, the packager, to fix some items and I expect you to do so to continue the review process. I reassign the review to myself. Anyway if you are not happy with my review, please speak up. -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1149649] Review Request: tuxfootball - funny soccer game
https://bugzilla.redhat.com/show_bug.cgi?id=1149649 Raphael Groner changed: What|Removed |Added Assignee|musur...@gmail.com |nob...@fedoraproject.org Whiteboard||NotReady --- Comment #3 from Raphael Groner --- If you notice some issues that need to be solved before you want to start a formal review, add these issues in a comment and set the Whiteboard of the bug to contain NotReady. This helps other possible reviewers to notice that the review request is not yet ready for further review action. https://fedoraproject.org/wiki/Package_Review_Process#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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1149649] Review Request: tuxfootball - funny soccer game
https://bugzilla.redhat.com/show_bug.cgi?id=1149649 --- Comment #2 from Raphael Groner --- Thanks a lot for your interest. I'll give some thought and come back later to the open issues. Sorry for the delay. -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1149649] Review Request: tuxfootball - funny soccer game
https://bugzilla.redhat.com/show_bug.cgi?id=1149649
Andrea Musuruane changed:
What|Removed |Added
Status|NEW |ASSIGNED
CC||musur...@gmail.com
Assignee|nob...@fedoraproject.org|musur...@gmail.com
Flags||fedora-review?
--- Comment #1 from Andrea Musuruane ---
Correct SRPM URL seem to be:
https://raphgro.fedorapeople.org/review/tuxfootball/tuxfootball-0.3.1-0.1.fc20.src.rpm
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
Better, more descriptive, summary: "Great 2D soccer (sometimes called football)
game"
License is wrong. It is "GPLv2+" (note the +) not "GPLv2".
Source0 URL is wrong. It should be:
http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net
In this way you can get rid of the %{ver} macro.
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
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.
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
Because the data package is build from the same source as the main package,
there is no real gain in splitting them apart.
You can use %{_pkgdocdir} instead of %{_docdir}/%{name}
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 package uses gettext for translations so you should add "BuildRequires:
gettext".
Moreover, you MUST use the %find_lang macro.
https://fedoraproject.org/wiki/Packaging:Guidelines#Handling_Locale_Files
Compilation is not verbose. Therefore it is not possible to check the compiler
flag used. Invoke make like "make %{?_smp_mflags} V=1"
Incorrect FSF addess should be reported upstream.
https://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address
tuxfootball-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/tuxfootball-0.3.1/src/SFont.c
tuxfootball-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/tuxfootball-0.3.1/src/SFont.h
tuxfootball-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/tuxfootball-0.3.1/src/Font.hpp
I'll perform a deeper analysis after these issues are fixed.
--
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
