https://bugzilla.redhat.com/show_bug.cgi?id=823873
Bug ID: 823873 QA Contact: extras-qa@fedoraproject.org Severity: medium Version: rawhide Priority: unspecified CC: notting@redhat.com, package-review@lists.fedoraproject.org Assignee: nobody@fedoraproject.org Summary: Review Request: ThePowderToy - falling sand physics sandbox game Regression: --- Story Points: --- Classification: Fedora OS: Linux Reporter: nazar@gmx.us Type: --- Documentation: --- Hardware: All Mount Type: --- Status: NEW Component: Package Review Product: Fedora
Spec URL: https://docs.google.com/open?id=0B_CR4N27LxsuM2NjU3JCci1wYk0 SRPM URL: https://docs.google.com/open?id=0B_CR4N27LxsuWGs3aDN4a2tfX3c Description: The Powder Toy is a desktop version of the classic 'falling sand' physics sandbox game, it simulates air pressure and velocity as well as heat! Fedora Account System Username: blasterblade Thats my first package, and I don't know where to host my files. Game is not mine, I am just a packager. Visit http://powdertoy.co.uk/ for details
https://bugzilla.redhat.com/show_bug.cgi?id=823873
Michael Cronenworth mike@cchtml.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mike@cchtml.com
--- Comment #1 from Michael Cronenworth mike@cchtml.com --- A few concerns:
Could you add the git checkout command used to create the tarball instead of "last git revision"?
Instead of using the git HEAD, could you ask upstream to put .tar.xz files on their Download Page for the stable releases? You could then use that tarball for your source tarball.
There is no .desktop file and most users would expect to find this listed in the "Games" menu. You should include one or ask upstream to create one.
https://bugzilla.redhat.com/show_bug.cgi?id=823873
--- Comment #2 from Nazar Mishturak nazar@gmx.us --- Latest stable release is 78.1. I used git clone git://github.com/FacialTurd/The-Powder-Toy.git from https://github.com/FacialTurd/The-Powder-Toy(their project page). So, I will now recompile for its stable version(it has less features :()
https://bugzilla.redhat.com/show_bug.cgi?id=823873
--- Comment #3 from Nazar Mishturak nazar@gmx.us --- Ok, I found the commit is 60e31ac78d1d59927a60c67d8d3e17929671dc18, version is 78.1(stable). I will now recompile. They don't use version numbers in git tag :(
https://bugzilla.redhat.com/show_bug.cgi?id=823873
--- Comment #4 from Michael Cronenworth mike@cchtml.com --- I'm not saying you are required to use the stable release, but I would recommend it if you are going to include this game in stable Fedora releases.
In any case, what we need is the git commit hash, or the checkout date, to reliably get the same source tarball that you created.
See the guidelines: http://fedoraproject.org/wiki/Packaging:SourceURL#Using_Revision_Control
https://bugzilla.redhat.com/show_bug.cgi?id=823873
--- Comment #5 from Nazar Mishturak nazar@gmx.us --- Well, I gave you the hash, but I made some minor changes to the code. Added a few defines(include file path) for it to compile and changed the Makefile(library name). Also I fixed parts in code where game saves it data. Now all of user data is stored in ~/.powder, not in current working directory. Do I need to post these changes? I can give you a diff to patch. Sorry If I am doing something wrong, it is my first time posting a package.
https://bugzilla.redhat.com/show_bug.cgi?id=823873
Michael Schwendt mschwendt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841
https://bugzilla.redhat.com/show_bug.cgi?id=823873
--- Comment #6 from Nazar Mishturak nazar@gmx.us --- Well It is done. It has a desktop file. Settings are under ~/.powder. Spec URL: https://docs.google.com/open?id=0B_CR4N27LxsuV2VLbGZPQng0dUE SRPM URL: https://docs.google.com/open?id=0B_CR4N27LxsuVWFfd254VXRSS1U x86_64 RPM: https://docs.google.com/open?id=0B_CR4N27LxsuUEt5eFppbmV0Q2s Source URL: https://docs.google.com/open?id=0B_CR4N27LxsuTzNXRFVxNTc4YWs
https://bugzilla.redhat.com/show_bug.cgi?id=823873
--- Comment #7 from Michael Cronenworth mike@cchtml.com --- Any changes made to upstream's source should be kept as patches and applied during RPM build time instead of inside your source tarball. You should also send the patches to upstream in hope that they apply them so it benefits everyone, and so you do not have to maintain them.
Ex: Patch0: settings.patch
%prep %setup -q %patch0 -p1
FYI: Since this is your first package and I am not a sponsor, I cannot review your package, but these comments will help speed up the process once a sponsor sees this bug. I am looking forward to this being in Fedora. :)
https://bugzilla.redhat.com/show_bug.cgi?id=823873
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |limburgher@gmail.com
--- Comment #8 from Jon Ciesla limburgher@gmail.com --- I am a sponsor and would be interesting in reviewing this and sponsoring you, have you looked at Michael's feedback from #7? Also, have you done any practice package reviews?
https://bugzilla.redhat.com/show_bug.cgi?id=823873
--- Comment #9 from Nazar Mishturak nazar@gmx.us --- Yes I looked at 7th comment. The patch is distro specific(changing compiler CFLAGS). You can see this at http://powdertoy.co.uk/Wiki/W/Compiling_for_Linux.html in Lua errors section. I haven't done any package reviews before, I am new here:). I don't know where I should put my desktop file? It is not in package git repo.
https://bugzilla.redhat.com/show_bug.cgi?id=823873
--- Comment #10 from Jon Ciesla limburgher@gmail.com --- Disto-specificness is exactly why it should be seperate. Source0 should be what you get from upstream. Then make a patch of your changes, and put Michael's Path0 line under the Source0 line, and the %patch0 line after %setup, as indicated. For the desktop file, you can use:
Source1: powder.desktop
And then at install, replace powder.desktop with %{SOURCE1}
Then have a look at:
https://fedoraproject.org/wiki/Packaging:Guidelines
You'll find lots of useful information.
https://bugzilla.redhat.com/show_bug.cgi?id=823873
Nazar Mishturak nazar@gmx.us changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |hdegoede@redhat.com Component|Package Review |pachi
--- Comment #11 from Nazar Mishturak nazar@gmx.us --- It's finished, but I dont know how I where I should upload my Source? files and patches?
https://bugzilla.redhat.com/show_bug.cgi?id=823873
Hans de Goede hdegoede@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC|hdegoede@redhat.com | Component|pachi |Package Review
https://bugzilla.redhat.com/show_bug.cgi?id=823873
--- Comment #12 from Nazar Mishturak nazar@gmx.us --- Nervermind that, I put them on my website. Package updated to version 81.2. All of Sources and Patch files are in http://blasterblade.x10.mx/packages/. SPEC file downloads them(from my website) SPEC URL: http://blasterblade.x10.mx/packages/ThePowderToy.spec SRPM URL: http://blasterblade.x10.mx/packages/ThePowderToy-81.2-1.fc17.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=823873
--- Comment #13 from Jon Ciesla limburgher@gmail.com --- Ok, better. If the Source and Patch files are generated by you, you can leave off the URL, so it should be:
Source0: ThePowderToy-81.2.tar.xz Source1: Powder.desktop Source2: powder.png Patch0: PowderMakefile.patch Patch1: mainc.patch
Also, there's nothing in the changelog. Make a changelog entry and increment Release every time you make a change.
Additionally, GPLv3+ is not the only license present. Run:
find . | xargs licensecheck
over the expanded tarball, there's also BSD, MIT, and Apache.
https://bugzilla.redhat.com/show_bug.cgi?id=823873
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(nazar@gmx.us)
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823873
Michael Cronenworth mike@cchtml.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841 (FE-NEEDSPONSOR) |201449 (FE-DEADREVIEW) Flags|needinfo?(nazar@gmx.us) |
--- Comment #14 from Michael Cronenworth mike@cchtml.com --- Per e-mail when I asked if he was going to continue this review:
"Sorry, I have to study now. Feel free to delete request.
Best regards, Nazar"
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823873
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution|--- |NOTABUG Last Closed| |2013-01-22 14:11:02
--- Comment #15 from Jon Ciesla limburgher@gmail.com --- Ok, thanks!
package-review@lists.fedoraproject.org